On Fri, Mar 18, 2016 at 6:08 PM, Ben Pfaff <b...@ovn.org> wrote:
> On Thu, Mar 03, 2016 at 12:13:24AM -0800, Andy Zhou wrote:
> > Add a global lock to serialize all OVSDB operations. It is a simple
> > locking scheme to implement and to reason about correctness, without
> > much changes to core of OVSDB implementation.
> >
> > Signed-off-by: Andy Zhou <az...@ovn.org>
>
> I don't understand yet what 'mutex' protects. Maybe it will become more
> clear as I read more patches.
>
> It protects all shared resources. For example, if any sessions shares
common 'db_mon' objects.
This lock effectively serializes all sessions request handling. It is
definitely an overkill, but easier
to get started with multithreading. I will add more comments about this.
Clang is irritating in that, if you fail to put the thread-safety
> annotations on both a function's prototype and its definition (if both
> are present), then it will not check every reference to the function.
> So here is an incremental diff that adds the annotations in the places
> they were missing. It also includes some stylistic changes.
>
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index dc38064..33cb0b2 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -51,7 +51,7 @@ struct ovsdb_jsonrpc_session;
> * method with an error. */
> static bool monitor2_enable__ = true;
>
> -/* A global lock to serilize just about all OVSDB operations. */
> +/* A global lock to serialize just about all OVSDB operations. */
> static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
>
> /* Message rate-limiting. */
> @@ -474,30 +474,33 @@ ovsdb_jsonrpc_server_use_threads(struct
> ovsdb_jsonrpc_server *svr) {
>
> struct ovsdb_jsonrpc_session {
> struct ovs_list node; /* Element in remote's sessions list. */
> - struct ovsdb_session up OVS_GUARDED_BY(mutex);
> + struct ovsdb_session up OVS_GUARDED_BY(mutex);
> const void *remote;
>
> /* Triggers. */
> /* Hmap of "struct ovsdb_jsonrpc_trigger"s. */
> - struct hmap triggers OVS_GUARDED_BY(mutex);
> + struct hmap triggers OVS_GUARDED_BY(mutex);
>
> /* Monitors. */
> /* Hmap of "struct ovsdb_jsonrpc_monitor"s. */
> - struct hmap monitors OVS_GUARDED_BY(mutex);
> + struct hmap monitors OVS_GUARDED_BY(mutex);
>
> /* Network connectivity. */
> struct jsonrpc_session *js; /* JSON-RPC session. */
> unsigned int js_seqno; /* Last jsonrpc_session_get_seqno()
> value. */
> };
>
> -static void ovsdb_jsonrpc_session_close(struct ovsdb_jsonrpc_session *);
> +static void ovsdb_jsonrpc_session_close(struct ovsdb_jsonrpc_session *)
> + OVS_EXCLUDED(mutex);
> static int ovsdb_jsonrpc_session_run(struct ovsdb_jsonrpc_session *)
> OVS_EXCLUDED(mutex);
> -static void ovsdb_jsonrpc_session_wait(struct ovsdb_jsonrpc_session *);
> +static void ovsdb_jsonrpc_session_wait(struct ovsdb_jsonrpc_session *)
> + OVS_EXCLUDED(mutex);
> static void ovsdb_jsonrpc_session_get_memory_usage(
> const struct ovsdb_jsonrpc_session *, struct simap *usage);
> static void ovsdb_jsonrpc_session_got_request(struct
> ovsdb_jsonrpc_session *,
> - struct jsonrpc_msg *);
> + struct jsonrpc_msg *)
> + OVS_REQUIRES(mutex);
> static void ovsdb_jsonrpc_session_got_notify(struct ovsdb_jsonrpc_session
> *,
> struct jsonrpc_msg *);
>
> @@ -524,6 +527,7 @@ ovsdb_jsonrpc_session_create(struct
> ovsdb_jsonrpc_remote *remote,
>
> static void
> ovsdb_jsonrpc_session_close(struct ovsdb_jsonrpc_session *s)
> + OVS_EXCLUDED(mutex)
> {
> struct ovsdb_jsonrpc_server *server;
>
> @@ -596,6 +600,7 @@ ovsdb_jsonrpc_session_set_options(struct
> ovsdb_jsonrpc_session *session,
>
> static void
> ovsdb_jsonrpc_session_wait(struct ovsdb_jsonrpc_session *s)
> + OVS_EXCLUDED(mutex)
> {
> jsonrpc_session_wait(s->js);
> if (!jsonrpc_session_get_backlog(s->js)) {
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
