Re: [PATCH v4 1/8] SUNRPC: introduce helpers for reference countedrpcbind clients
From: Stanislav Kinsbursky
Date: Tue Sep 20 2011 - 13:27:06 EST
20.09.2011 21:13, Myklebust, Trond ÐÐÑÐÑ:
-----Original Message-----
From: Stanislav Kinsbursky [mailto:skinsbursky@xxxxxxxxxxxxx]
Sent: Tuesday, September 20, 2011 12:21 PM
To: Myklebust, Trond
Cc: Schumaker, Bryan; linux-nfs@xxxxxxxxxxxxxxx; Pavel Emelianov;
neilb@xxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
bfields@xxxxxxxxxxxx; davem@xxxxxxxxxxxxx
Subject: Re: [PATCH v4 1/8] SUNRPC: introduce helpers for reference
counted rpcbind clients
20.09.2011 18:38, Myklebust, Trond ÐÐÑÐÑ:
-----Original Message-----
From: Stanislav Kinsbursky [mailto:skinsbursky@xxxxxxxxxxxxx]
Sent: Tuesday, September 20, 2011 10:35 AM
To: Myklebust, Trond
Cc: Schumaker, Bryan; linux-nfs@xxxxxxxxxxxxxxx; Pavel Emelianov;
neilb@xxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
bfields@xxxxxxxxxxxx; davem@xxxxxxxxxxxxx
Subject: Re: [PATCH v4 1/8] SUNRPC: introduce helpers for reference
counted rpcbind clients
20.09.2011 18:14, Myklebust, Trond ÐÐÑÐÑ:
Doesn't it need to be protected by rpcb_clnt_lock too?
Nope from my pow. It's protected by rpcb_create_local_mutex. I.e.
no one will change rpcb_users since it's zero. If it's non zero -
we willn't get to rpcb_set_local().
OK, so you are saying that the rpcb_users++ below could be replaced
by
rpcb_users=1?
Yes, you right.
In that case, don't you need a smp_wmb() between the setting of
rpcb_local_clnt/4 and the setting of rpcb_users? Otherwise, how do
you guarantee that rpcb_users != 0 implies rpbc_local_clnt/4 != NULL?
We check rpcb_users under spinlock. Gain spinlock forces memory
barrier, doesn't it?
Yes, and so you don't need an smp_rmb() on the reader side. However,
you still need to ensure that the processor which _sets_ the rpcb_users and
rpcb_local_clnt/4 actually writes them in the correct order.
Trond, I've thought again and realized, that even if these writes (rpcb_users
and rpbc_local_clnt/4) will be done in reversed order, then no barrier
required here.
Because if we have another process trying to create those clients (it can't use
them since it's not started yet) on other CPU, than it's waiting on creation
mutex. When it will gain the mutex, we will have required memory barrier
for us.
Or I missed something in your explanation?
You need to ensure that if someone calls rpcb_get_local() and gets a positive result, then they can rely on rpcb_local_clnt/4 being non-zero. Without the write barrier, that is not the case.
In current context we can be sure, that between rpcb_get_local() and first
dereference of rpcb_local_clnt/4 we have at least one spinlock
(svc_xprt_class_lock in svc_create_xprt).
But I understand, that we can't relay on it since this code can be changed in
future.
So I'll add barrier there.
Without that guarantee, you can't really ensure that rpcb_put_local() will work as expected either, since it will be possible to trigger the --rpcb_users == 0 case without shutting down rpcb_local_clnt/4 (because clnt/clnt4 == 0).
Yes, you right. But it doesn't mean, that we require barrier here, because we
don't need this garantee you are talking about.
We can be sure, that we always see right rpcb_users value. It means that, if we
set this value to zero, then no other running services left and no references to
those clients can occur.
And even if we have another process which is going to create new service right
now on another CPU, then this process will see that no rpcbind users present and
will create new clients and assign them to global variables prior to any use of
this clients can occur.
And this assign will be done with barrier as we agreed earlier.
Cheers
Trond
--
Best regards,
Stanislav Kinsbursky
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/