On Mon, Dec 30, 2013 at 03:18:15PM -0800, Pravin Shelar wrote:
> On Fri, Dec 27, 2013 at 8:03 PM, Ben Pfaff <b...@nicira.com> wrote:
> > This is a first step in making thread safety more granular in dpif-netdev,
> > to allow for multithreaded forwarding.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > lib/dpif-netdev.c | 54
> > ++++++++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 37 insertions(+), 17 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index d0f6dab..727483a 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -83,8 +83,9 @@ struct dp_netdev_upcall {
> > };
> >
> > struct dp_netdev_queue {
> > - struct dp_netdev_upcall upcalls[MAX_QUEUE_LEN];
> > - unsigned int head, tail;
> > + struct dp_netdev_upcall upcalls[MAX_QUEUE_LEN] OVS_GUARDED;
> > + unsigned int head OVS_GUARDED;
> > + unsigned int tail OVS_GUARDED;
> > };
> >
> > /* Datapath based on the network device interface from netdev.h. */
> > @@ -94,9 +95,12 @@ struct dp_netdev {
> > struct ovs_refcount ref_cnt;
> > atomic_flag destroyed;
> >
> > - struct dp_netdev_queue queues[N_QUEUES];
> > struct classifier cls; /* Classifier. */
> > struct hmap flow_table; /* Flow table. */
> > +
> > + /* Queues. */
> > + struct ovs_mutex queue_mutex;
> > + struct dp_netdev_queue queues[N_QUEUES];
> > struct seq *queue_seq; /* Incremented whenever a packet is
> > queued. */
> >
>
> It is just more clear if we move the lock inside dp_netdev_queue since
> it is protecting head and tail. Is there any reason not to do it?
> But I am fine either way.
I put it at a higher level because of find_nonempty_queue(), which is
supposed to report whether a packet is available in *any* queue. If
the lock were per-queue then it would have to take all the locks. It
seemed simpler, to me, to use a single lock.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
