From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1FC5DC32753 for ; Wed, 14 Aug 2019 14:15:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E7011214DA for ; Wed, 14 Aug 2019 14:15:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1565792112; bh=kburXRh82xHoH2ULBf4qbUaBruG20vqIvfvEbFcltJY=; h=Subject:From:To:Cc:Date:In-Reply-To:References:List-ID:From; b=ILeN6J6UQ9UlToF+b2Gh3l815ZvVfukDCOnig1+ho3+o8E1rghL9A1m/IxB7tDM0s BrCrLII9gwrqjZBuCZODt5bo/tUo1NZ21m2H9Tycs6Wl5IuxSddcdMI9MSo740qu4U GwGckpF+ajroDHOq8S7EYbvgQ1Y6SblISHHTqsBA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727461AbfHNOPL (ORCPT ); Wed, 14 Aug 2019 10:15:11 -0400 Received: from mail.kernel.org ([198.145.29.99]:50896 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726019AbfHNOPK (ORCPT ); Wed, 14 Aug 2019 10:15:10 -0400 Received: from tleilax.poochiereds.net (68-20-15-154.lightspeed.rlghnc.sbcglobal.net [68.20.15.154]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id F11AF2133F; Wed, 14 Aug 2019 14:15:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1565792109; bh=kburXRh82xHoH2ULBf4qbUaBruG20vqIvfvEbFcltJY=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=YmMNPxPAWp25eQhuqgZ4ltWhMxF28BvGB5vskr14XqYVCQZZ96TVxfUdULrpt9Fg0 60trd+Jkb520pjcYaqrosDqW5ftToo6hZAXtZVmiRfDVvso+e6i9CkZS/dVjFp3R2p sxNd0hLUe0BkrhCT6Rwkhd2HlJfkAotXSoeuAaW0= Message-ID: Subject: Re: [RFC PATCH v2 02/19] fs/locks: Add Exclusive flag to user Layout lease From: Jeff Layton To: ira.weiny@intel.com, Andrew Morton Cc: Jason Gunthorpe , Dan Williams , Matthew Wilcox , Jan Kara , Theodore Ts'o , John Hubbard , Michal Hocko , Dave Chinner , linux-xfs@vger.kernel.org, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-nvdimm@lists.01.org, linux-ext4@vger.kernel.org, linux-mm@kvack.org Date: Wed, 14 Aug 2019 10:15:06 -0400 In-Reply-To: <20190809225833.6657-3-ira.weiny@intel.com> References: <20190809225833.6657-1-ira.weiny@intel.com> <20190809225833.6657-3-ira.weiny@intel.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.32.4 (3.32.4-1.fc30) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2019-08-09 at 15:58 -0700, ira.weiny@intel.com wrote: > From: Ira Weiny > > Add an exclusive lease flag which indicates that the layout mechanism > can not be broken. > > Exclusive layout leases allow the file system to know that pages may be > GUP pined and that attempts to change the layout, ie truncate, should be > failed. > > A process which attempts to break it's own exclusive lease gets an > EDEADLOCK return to help determine that this is likely a programming bug > vs someone else holding a resource. > > Signed-off-by: Ira Weiny > --- > fs/locks.c | 23 +++++++++++++++++++++-- > include/linux/fs.h | 1 + > include/uapi/asm-generic/fcntl.h | 2 ++ > 3 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index ad17c6ffca06..0c7359cdab92 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -626,6 +626,8 @@ static int lease_init(struct file *filp, long type, unsigned int flags, > fl->fl_flags = FL_LEASE; > if (flags & FL_LAYOUT) > fl->fl_flags |= FL_LAYOUT; > + if (flags & FL_EXCLUSIVE) > + fl->fl_flags |= FL_EXCLUSIVE; > fl->fl_start = 0; > fl->fl_end = OFFSET_MAX; > fl->fl_ops = NULL; > @@ -1619,6 +1621,14 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type) > list_for_each_entry_safe(fl, tmp, &ctx->flc_lease, fl_list) { > if (!leases_conflict(fl, new_fl)) > continue; > + if (fl->fl_flags & FL_EXCLUSIVE) { > + error = -ETXTBSY; > + if (new_fl->fl_pid == fl->fl_pid) { > + error = -EDEADLOCK; > + goto out; > + } > + continue; > + } > if (want_write) { > if (fl->fl_flags & FL_UNLOCK_PENDING) > continue; > @@ -1634,6 +1644,13 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type) > locks_delete_lock_ctx(fl, &dispose); > } > > + /* We differentiate between -EDEADLOCK and -ETXTBSY so the above loop > + * continues with -ETXTBSY looking for a potential deadlock instead. > + * If deadlock is not found go ahead and return -ETXTBSY. > + */ > + if (error == -ETXTBSY) > + goto out; > + > if (list_empty(&ctx->flc_lease)) > goto out; > > @@ -2044,9 +2061,11 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg) > * to revoke the lease in break_layout() And this is done by using > * F_WRLCK in the break code. > */ > - if (arg == F_LAYOUT) { > + if ((arg & F_LAYOUT) == F_LAYOUT) { > + if ((arg & F_EXCLUSIVE) == F_EXCLUSIVE) > + flags |= FL_EXCLUSIVE; > arg = F_RDLCK; > - flags = FL_LAYOUT; > + flags |= FL_LAYOUT; > } > > fl = lease_alloc(filp, arg, flags); > diff --git a/include/linux/fs.h b/include/linux/fs.h > index dd60d5be9886..2e41ce547913 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1005,6 +1005,7 @@ static inline struct file *get_file(struct file *f) > #define FL_UNLOCK_PENDING 512 /* Lease is being broken */ > #define FL_OFDLCK 1024 /* lock is "owned" by struct file */ > #define FL_LAYOUT 2048 /* outstanding pNFS layout or user held pin */ > +#define FL_EXCLUSIVE 4096 /* Layout lease is exclusive */ > > #define FL_CLOSE_POSIX (FL_POSIX | FL_CLOSE) > > diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h > index baddd54f3031..88b175ceccbc 100644 > --- a/include/uapi/asm-generic/fcntl.h > +++ b/include/uapi/asm-generic/fcntl.h > @@ -176,6 +176,8 @@ struct f_owner_ex { > > #define F_LAYOUT 16 /* layout lease to allow longterm pins such as > RDMA */ > +#define F_EXCLUSIVE 32 /* layout lease is exclusive */ > + /* FIXME or shoudl this be F_EXLCK??? */ > > /* operations for bsd flock(), also used by the kernel implementation */ > #define LOCK_SH 1 /* shared lock */ This interface just seems weird to me. The existing F_*LCK values aren't really set up to be flags, but are enumerated values (even if there are some gaps on some arches). For instance, on parisc and sparc: /* for posix fcntl() and lockf() */ #define F_RDLCK 01 #define F_WRLCK 02 #define F_UNLCK 03 While your new flag values are well above these values, it's still a bit sketchy to do what you're proposing from a cross-platform interface standpoint. I think this would be a lot cleaner if you weren't overloading the F_SETLEASE command with new flags, and instead added new F_SETLAYOUT/F_GETLAYOUT cmd values. You'd then be free to define a new set of "arg" values for use with layouts, and there's be a clear distinction interface-wise between setting a layout and a lease. -- Jeff Layton