From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Layton Subject: Re: [PATCH 1/2] locks: add missing memory barrier in break_deleg Date: Tue, 10 Jun 2014 17:05:18 -0400 Message-ID: <20140610170518.4854cdd3@f20.localdomain> References: <1402431276-14350-1-git-send-email-jlayton@poochiereds.net> <1402431276-14350-2-git-send-email-jlayton@poochiereds.net> <20140610204828.GL3957@fieldses.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: linux-fsdevel@vger.kernel.org To: "J. Bruce Fields" Return-path: Received: from mail-ie0-f170.google.com ([209.85.223.170]:37002 "EHLO mail-ie0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751998AbaFJVFY (ORCPT ); Tue, 10 Jun 2014 17:05:24 -0400 Received: by mail-ie0-f170.google.com with SMTP id tr6so2896096ieb.15 for ; Tue, 10 Jun 2014 14:05:23 -0700 (PDT) In-Reply-To: <20140610204828.GL3957@fieldses.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, 10 Jun 2014 16:48:28 -0400 "J. Bruce Fields" 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 > > 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 > > --- > > 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