> -----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. 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). Cheers Trond ��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥