On Tue, 2012-12-11 at 15:32 +0000, Adamson, Andy wrote:
> On Dec 10, 2012, at 3:02 PM, "Myklebust, Trond" <Trond.Myklebust@xxxxxxxxxx> wrote:
>
> > On Fri, 2012-11-30 at 13:09 -0500, andros@xxxxxxxxxx wrote:
> >> From: Andy Adamson <andros@xxxxxxxxxx>
> >>
> >> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
> >> ---
> >> A cleanup patch.
> >>
> >> net/sunrpc/auth.c | 1 +
> >> net/sunrpc/auth_gss/auth_gss.c | 1 +
> >> 2 files changed, 2 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> >> index b5c067b..8b76753 100644
> >> --- a/net/sunrpc/auth.c
> >> +++ b/net/sunrpc/auth.c
> >> @@ -225,6 +225,7 @@ rpcauth_unhash_cred_locked(struct rpc_cred *cred)
> >> hlist_del_rcu(&cred->cr_hash);
> >> smp_mb__before_clear_bit();
> >> clear_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags);
> >> + smp_mb__after_clear_bit();
> >> }
> >>
> >> static int
> >> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> >> index 909dc0c..92a7d6d 100644
> >> --- a/net/sunrpc/auth_gss/auth_gss.c
> >> +++ b/net/sunrpc/auth_gss/auth_gss.c
> >> @@ -126,6 +126,7 @@ gss_cred_set_ctx(struct rpc_cred *cred, struct gss_cl_ctx *ctx)
> >> set_bit(RPCAUTH_CRED_UPTODATE, &cred->cr_flags);
> >> smp_mb__before_clear_bit();
> >> clear_bit(RPCAUTH_CRED_NEW, &cred->cr_flags);
> >> + smp_mb__after_clear_bit();
> >> }
> >>
> >> static const void *
> >
> > Why are these needed?
>
> AFAICS the documentation says to either call both smp_mb__before_clear_bit and smp_mp__after_clear_bit to ensure that memory operations before and after the clear_bit call are strongly ordered, or don't call either. Since there can be multiple threads looking at a gss_cred cr_flags, it seems to me that we want the memory barriers to get the correct value of the bit.
No. It doesn't say do either both or nothing. It says that if you call
smp_mb__after_clear_bit(), then the clear_bit() will be strongly ordered
w.r.t. future memory operations done by this thread. My question is why
we need this strong ordering?
The current code already orders the hlist_del_rcu() and the
set_bit(RPCAUTH_CRED_UPTODATE) w.r.t. the clear_bit(), so any thread
that sees the RPCAUTH_CRED_HASHED/RPCAUTH_CRED_NEW bits cleared will
also see the list removal/uptodate cred.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
