[lustre-devel] [PATCH 17/30] lustre: llite: protect ll_dentry_data modification

[James Simmons]

> 
> > From: Andriy Skulysh <c17819 at cray.com>
> >
> > The ll_dentry_data bitfields modification should be protected by
> > a spinlock.
> >
> > Signed-off-by: Andriy Skulysh <c17819 at cray.com>
> > Signed-off-by: Lai Siyao <lai.siyao at whamcloud.com>
> > WC-bug-id: https://jira.whamcloud.com/browse/LU-9241
> > Seagate-bug-id: MRP-4257
> > Reviewed-on: https://review.whamcloud.com/26109
> > Reviewed-by: Niu Yawei <yawei.niu at intel.com>
> > Reviewed-by: Bobi Jam <bobijam at hotmail.com>
> > Reviewed-by: Oleg Drokin <green at whamcloud.com>
> > Signed-off-by: James Simmons <jsimmons at infradead.org>
> > ---
> >  drivers/staging/lustre/lustre/llite/llite_nfs.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/llite/llite_nfs.c b/drivers/staging/lustre/lustre/llite/llite_nfs.c
> > index c66072a..5e91e83 100644
> > --- a/drivers/staging/lustre/lustre/llite/llite_nfs.c
> > +++ b/drivers/staging/lustre/lustre/llite/llite_nfs.c
> > @@ -171,8 +171,9 @@ struct lustre_nfs_fid {
> >  	 * we came from NFS and so opencache needs to be
> >  	 * enabled for this one
> >  	 */
> > +	spin_lock(&result->d_lock);
> >  	ll_d2d(result)->lld_nfs_dentry = 1;
> > -
> > +	spin_unlock(&result->d_lock);
> >  	return result;
> >  }
> 
> This is a bit weird....
> I agree that having a spinlock is needed as the compiler does an R/M/W
> to update the bitfield, and that could race with updats to lld_invalid
> (and I think that is a good arguement to avoid bitfields in shared structures).
> 
> However lld_nfs_dentry is never used.  The only use was removed by
> 
> Commit: c1b66fccf986 ("staging: lustre: fid: do open-by-fid by default")
> 
> The corresponding commit upstream is
> 
> Commit: cb85c0364fd8 ("LU-3544 fid: do open-by-fid by default")
> 
> and that commit doesn't remove the use of lld_nfs_dentry.  Maybe because
> lld_nfs_dentry didn't exist then - it wasn't added until later
> by 65d0add6057b138.
> 
> So do we need to bring lld_nfs_dentry back?

Ouch. That got removed by mistake by me ;-( Yes it should come back. 
I will create a patch to fix that. My bad.