Thanks for the review!

> On Jul 1, 2016, at 9:08 PM, Ben Pfaff <b...@ovn.org> wrote:
> 
> On Tue, Jun 21, 2016 at 05:18:45PM -0700, Jarno Rajahalme wrote:
>> PMD threads use pvectors but do not need the overhead of the
>> concurrent version.  Expose the internal non-concurrent version for
>> that use.
>> 
>> Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
> 
> Seems like a good idea, if we have a potential user (do we?).
> 

I just posted a v2 that includes the user in dpif-netdev.c.

> I guess that the following comments are actually relevant with or
> without this patch.
> 

I added separate patches for these

> Any reason why PVECTOR_EXTRA_ALLOC is in pvector.h instead of pvector.c?
> 
> Any reason we couldn't do the following, instead of null-ing in-place?
> I think that we have to sort the vector either way, and then we could
> get rid of the INT_MIN special case.
> 

I had to slightly change the semantics of PVECTOR_FOR_EACH_PRIORITY to do this.

> diff --git a/lib/pvector.c b/lib/pvector.c
> index 00880bd..55e5351 100644
> --- a/lib/pvector.c
> +++ b/lib/pvector.c
> @@ -158,10 +158,7 @@ pvector_impl_remove(struct pvector_impl *impl, void *ptr)
> 
>     index = pvector_impl_find(impl, ptr);
>     ovs_assert(index >= 0);
> -    /* Now at the index of the entry to be deleted.
> -     * Clear in place, sort will clean these off. */
> -    impl->vector[index].ptr = NULL;
> -    impl->vector[index].priority = INT_MIN;
> +    impl->vector[index] = impl->vector[--impl->size];
> }
> 
> void
> 
> Acked-by: Ben Pfaff <b...@ovn.org>

I also renamed pvector to cpvector and pvector_impl to pvector. It is pretty 
straightforward, but I'd like you to scan the changes once more, so I posted a 
v2.

  Jarno


_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to