Re: [PATCH 1/2] locks: add missing memory barrier in break_deleg

[Jeff Layton <jlayton@poochiereds.net>]

On Tue, 10 Jun 2014 16:48:28 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Tue, Jun 10, 2014 at 04:14:35PM -0400, Jeff Layton wrote:
> > break_deleg is subject to the same potential race as break_lease. Add
> > a memory barrier to prevent it.
> 
> Acked-by: J. Bruce Fields <bfields@redhat.com>
> 
> Though we might now just move the comment, smp_mb(), and inode->i_flock
> calls into __break_lease?
> 
> --b.
> 

We certainly could, but it would mean they'd no longer be inlined. I
doubt it'd make much difference, but I'm inclined to leave them as
inlines for now.

> > 
> > Signed-off-by: Jeff Layton <jlayton@primarydata.com>
> > ---
> >  include/linux/fs.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index c3f46e499dd0..22ae79650b82 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1914,6 +1914,12 @@ static inline int break_lease(struct inode *inode, unsigned int mode)
> >  
> >  static inline int break_deleg(struct inode *inode, unsigned int mode)
> >  {
> > +	/*
> > +	 * Since this check is lockless, we must ensure that any refcounts
> > +	 * taken are done before checking inode->i_flock. Otherwise, we could
> > +	 * end up racing with tasks trying to set a new lease on this file.
> > +	 */
> > +	smp_mb();
> >  	if (inode->i_flock)
> >  		return __break_lease(inode, mode, FL_DELEG);
> >  	return 0;
> > -- 
> > 1.9.3
> > 


-- 
Jeff Layton <jlayton@poochiereds.net>