On Wed, Mar 16, 2016 at 11:43 AM, Ryan Moats <rmo...@us.ibm.com> wrote:
> > Andy Zhou <az...@ovn.org> wrote on 03/16/2016 01:20:48 PM: > > > From: Andy Zhou <az...@ovn.org> > > To: Ryan Moats/Omaha/IBM@IBMUS > > Cc: ovs dev <dev@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 <rmo...@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 <az...@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@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
