On Wed, Jul 17, 2013 at 5:03 PM, Ansis Atteka <aatt...@nicira.com> wrote:
>
>
>
> On Fri, Jul 12, 2013 at 2:54 PM, Ben Pfaff <b...@nicira.com> wrote:
>
>> The uses of vlog in this module are not thread-safe, because vlog itself
>> is not yet thread-safe.
>>
>> Signed-off-by: Ben Pfaff <b...@nicira.com>
>> ---
>> lib/netlink-socket.c | 24 +++++++++++++++++++-----
>> lib/netlink-socket.h | 6 ++++++
>> 2 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
>> index 8e08841..da32284 100644
>> --- a/lib/netlink-socket.c
>> +++ b/lib/netlink-socket.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
>> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>> *
>> * Licensed under the Apache License, Version 2.0 (the "License");
>> * you may not use this file except in compliance with the License.
>> @@ -29,6 +29,7 @@
>> #include "netlink.h"
>> #include "netlink-protocol.h"
>> #include "ofpbuf.h"
>> +#include "ovs-thread.h"
>> #include "poll-loop.h"
>> #include "socket-util.h"
>> #include "util.h"
>> @@ -85,13 +86,14 @@ static void nl_pool_release(struct nl_sock *);
>> int
>> nl_sock_create(int protocol, struct nl_sock **sockp)
>> {
>> + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> struct nl_sock *sock;
>> struct sockaddr_nl local, remote;
>> socklen_t local_size;
>> int rcvbuf;
>> int retval = 0;
>>
>> - if (!max_iovs) {
>> + if (ovsthread_once_start(&once)) {
>>
>
> I could be wrong, but isn't max_iovs shared among all threads and hence
> should be protected too? I guess the code in this "if" statement could
> potentially be executed simultaneously by multiple threads until
> ovsthread_once_done() is called? is that right?
>
Nevermind, I guess I misunderstood ovsthread_once* API in a silly way.
Looks good to me.
>
>
> Other than that looks good to me.
>
> int save_errno = errno;
>> errno = 0;
>>
>> @@ -106,6 +108,7 @@ nl_sock_create(int protocol, struct nl_sock **sockp)
>> }
>>
>> errno = save_errno;
>> + ovsthread_once_done(&once);
>> }
>>
>> *sockp = NULL;
>> @@ -1010,17 +1013,25 @@ struct nl_pool {
>> };
>>
>> static struct nl_pool pools[MAX_LINKS];
>> +static pthread_mutex_t pool_mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER;
>>
>> static int
>> nl_pool_alloc(int protocol, struct nl_sock **sockp)
>> {
>> + struct nl_sock *sock = NULL;
>> struct nl_pool *pool;
>>
>> ovs_assert(protocol >= 0 && protocol < ARRAY_SIZE(pools));
>>
>> + xpthread_mutex_lock(&pool_mutex);
>> pool = &pools[protocol];
>> if (pool->n > 0) {
>> - *sockp = pool->socks[--pool->n];
>> + sock = pool->socks[--pool->n];
>> + }
>> + xpthread_mutex_unlock(&pool_mutex);
>> +
>> + if (sock) {
>> + *sockp = sock;
>> return 0;
>> } else {
>> return nl_sock_create(protocol, sockp);
>> @@ -1033,11 +1044,14 @@ nl_pool_release(struct nl_sock *sock)
>> if (sock) {
>> struct nl_pool *pool = &pools[sock->protocol];
>>
>> + xpthread_mutex_lock(&pool_mutex);
>> if (pool->n < ARRAY_SIZE(pool->socks)) {
>> pool->socks[pool->n++] = sock;
>> - } else {
>> - nl_sock_destroy(sock);
>> + sock = NULL;
>> }
>> + xpthread_mutex_unlock(&pool_mutex);
>> +
>> + nl_sock_destroy(sock);
>> }
>> }
>>
>> diff --git a/lib/netlink-socket.h b/lib/netlink-socket.h
>> index c77050e..986b574 100644
>> --- a/lib/netlink-socket.h
>> +++ b/lib/netlink-socket.h
>> @@ -30,6 +30,12 @@
>> * are Linux-specific. For Netlink protocol definitions, see
>> * netlink-protocol.h. For helper functions for working with Netlink
>> messages,
>> * see netlink.h.
>> + *
>> + *
>> + * Thread-safety
>> + * =============
>> + *
>> + * Only a single thread may use a given nl_sock or nl_dump at one time.
>> */
>>
>> #include <stdbool.h>
>> --
>> 1.7.2.5
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
