[ovs-dev] [ovs-dev, ovsdb-server, multithreading, RFC, 1/9] ovsdb: Do not group sessions by remote.

Andy Zhou azhou at ovn.org
Wed Mar 16 19:43:40 UTC 2016


On Wed, Mar 16, 2016 at 11:43 AM, Ryan Moats <rmoats at us.ibm.com> wrote:

>
> Andy Zhou <azhou at ovn.org> wrote on 03/16/2016 01:20:48 PM:
>
> > From: Andy Zhou <azhou at ovn.org>
> > To: Ryan Moats/Omaha/IBM at IBMUS
> > Cc: ovs dev <dev at openvswitch.org>
> > Date: 03/16/2016 01:31 PM
> > Subject: Re: [ovs-dev] [ovs-dev, ovsdb-server, multithreading, RFC,
> > 1/9] ovsdb: Do not group sessions by remote.
> >
> > On Wed, Mar 16, 2016 at 10:38 AM, Ryan Moats <rmoats at us.ibm.com> wrote:
> >
> > ---- Original Message ----
> > > Currently ovsdb_jsonrpc_session are grouped together in a linked
> > > list within  'ovsdb_jsonrpc_remote'. This makes sense since most
> > > session operations applies to sessions within a remote.
> > >
> > > However, in order to scale up ovsdb-server with multi-threading, it is
> > > more convenient to distribute a sessions to any thread available,
> > > regardless which remote it is associated with.
> > >
> > > This patch introduces a set of APIs that provide operations on
> > > a list of sessions. Instead of group sessions by remote, they
> > > are linked together in a new ovs_list field 'all_sessions' in the
> > > ovsdb_jsonrpc_server struct.
> > >
> > > With multi-threading, the design is that all sessions managed
> > > by a thread will have them linked together on a thread private
> > > linked list. At that time, the 'all_sessions' field in
> > > ovsdb_jsonrpc_server struct will have all session managed
> > > the main process.
> > >
> > > Signed-off-by: Andy Zhou <azhou at ovn.org>
> >
> > I've given this patch a bit of a test spin and while it appears
> > relatively sane for simple cases, I'm concerned that the patch
> > should be using LIST_FOR_EACH_SAFE in more places than it currently
> > does. The code snippet that got me wondering was the use of
> > LIST_FOR_EACH in ovsdb_jsonrpc_sessions_count()...\
> >
> > Thanks for testing it.
> >
> > LIST_FOR_EACH_SAFE is required to protect against possible list elements
> > removal during the list traversal. Since a session is always
> > assigned to a single thread,
> > I don't see how it can be remove from the list during the
> > ovsdb_jsonprc_session_count(),
> > Or I am still missing something here.
>
> I'm probably misreading, but I read the all_sessions list as being
> accessible by all threads.  That means I have to worry about thread 1
> removing a session from all_sessions while thread 2 is within
> sessions_count.
>

Threads are not introduced with this patch.

When threads are addded in 9/3 the comment for all_sessions is updated.
It holds sessions created by the main process.


>
> Ryan (regXboi)
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list