All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@primarydata.com>
To: linux-fsdevel@vger.kernel.org
Cc: linux-nfs@vger.kernel.org, Christoph Hellwig <hch@infradead.org>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 05/17] locks: generic_delete_lease doesn't need a file_lock at all
Date: Thu,  4 Sep 2014 08:38:31 -0400	[thread overview]
Message-ID: <1409834323-7171-6-git-send-email-jlayton@primarydata.com> (raw)
In-Reply-To: <1409834323-7171-1-git-send-email-jlayton@primarydata.com>

Ensure that it's OK to pass in a NULL file_lock double pointer on
a F_UNLCK request and convert the vfs_setlease F_UNLCK callers to
do just that.

Finally, turn the BUG_ON in generic_setlease into a WARN_ON_ONCE
with an error return. That's a problem we can handle without
crashing the box if it occurs.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/locks.c                      | 34 ++++++++++++++--------------------
 fs/nfsd/nfs4state.c             |  2 +-
 include/trace/events/filelock.h | 14 +++++++-------
 3 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 4031324e6cca..1289b74fffbf 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1637,22 +1637,23 @@ out:
 	return error;
 }
 
-static int generic_delete_lease(struct file *filp, struct file_lock **flp)
+static int generic_delete_lease(struct file *filp)
 {
+	int error = -EAGAIN;
 	struct file_lock *fl, **before;
 	struct dentry *dentry = filp->f_path.dentry;
 	struct inode *inode = dentry->d_inode;
 
-	trace_generic_delete_lease(inode, *flp);
-
 	for (before = &inode->i_flock;
 			((fl = *before) != NULL) && IS_LEASE(fl);
 			before = &fl->fl_next) {
-		if (fl->fl_file != filp)
-			continue;
-		return (*flp)->fl_lmops->lm_change(before, F_UNLCK);
+		if (fl->fl_file == filp)
+			break;
 	}
-	return -EAGAIN;
+	trace_generic_delete_lease(inode, fl);
+	if (fl)
+		error = fl->fl_lmops->lm_change(before, F_UNLCK);
+	return error;
 }
 
 /**
@@ -1682,13 +1683,15 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp)
 
 	time_out_leases(inode);
 
-	BUG_ON(!(*flp)->fl_lmops->lm_break);
-
 	switch (arg) {
 	case F_UNLCK:
-		return generic_delete_lease(filp, flp);
+		return generic_delete_lease(filp);
 	case F_RDLCK:
 	case F_WRLCK:
+		if (!(*flp)->fl_lmops->lm_break) {
+			WARN_ON_ONCE(1);
+			return -ENOLCK;
+		}
 		return generic_add_lease(filp, arg, flp);
 	default:
 		return -EINVAL;
@@ -1744,15 +1747,6 @@ int vfs_setlease(struct file *filp, long arg, struct file_lock **lease)
 }
 EXPORT_SYMBOL_GPL(vfs_setlease);
 
-static int do_fcntl_delete_lease(struct file *filp)
-{
-	struct file_lock fl, *flp = &fl;
-
-	lease_init(filp, F_UNLCK, flp);
-
-	return vfs_setlease(filp, F_UNLCK, &flp);
-}
-
 static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
 {
 	struct file_lock *fl, *ret;
@@ -1809,7 +1803,7 @@ out_unlock:
 int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
 {
 	if (arg == F_UNLCK)
-		return do_fcntl_delete_lease(filp);
+		return vfs_setlease(filp, F_UNLCK, NULL);
 	return do_fcntl_add_lease(fd, filp, arg);
 }
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 29fac18d9102..0cd252916e1a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -683,7 +683,7 @@ static void nfs4_put_deleg_lease(struct nfs4_file *fp)
 	if (!fp->fi_lease)
 		return;
 	if (atomic_dec_and_test(&fp->fi_delegees)) {
-		vfs_setlease(fp->fi_deleg_file, F_UNLCK, &fp->fi_lease);
+		vfs_setlease(fp->fi_deleg_file, F_UNLCK, NULL);
 		fp->fi_lease = NULL;
 		fput(fp->fi_deleg_file);
 		fp->fi_deleg_file = NULL;
diff --git a/include/trace/events/filelock.h b/include/trace/events/filelock.h
index 59d11c22f076..a0d008070962 100644
--- a/include/trace/events/filelock.h
+++ b/include/trace/events/filelock.h
@@ -53,15 +53,15 @@ DECLARE_EVENT_CLASS(filelock_lease,
 	),
 
 	TP_fast_assign(
-		__entry->fl = fl;
+		__entry->fl = fl ? fl : NULL;
 		__entry->s_dev = inode->i_sb->s_dev;
 		__entry->i_ino = inode->i_ino;
-		__entry->fl_next = fl->fl_next;
-		__entry->fl_owner = fl->fl_owner;
-		__entry->fl_flags = fl->fl_flags;
-		__entry->fl_type = fl->fl_type;
-		__entry->fl_break_time = fl->fl_break_time;
-		__entry->fl_downgrade_time = fl->fl_downgrade_time;
+		__entry->fl_next = fl ? fl->fl_next : NULL;
+		__entry->fl_owner = fl ? fl->fl_owner : NULL;
+		__entry->fl_flags = fl ? fl->fl_flags : 0;
+		__entry->fl_type = fl ? fl->fl_type : 0;
+		__entry->fl_break_time = fl ? fl->fl_break_time : 0;
+		__entry->fl_downgrade_time = fl ? fl->fl_downgrade_time : 0;
 	),
 
 	TP_printk("fl=0x%p dev=0x%x:0x%x ino=0x%lx fl_next=0x%p fl_owner=0x%p fl_flags=%s fl_type=%s fl_break_time=%lu fl_downgrade_time=%lu",
-- 
1.9.3


  parent reply	other threads:[~2014-09-04 12:39 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-04 12:38 [PATCH v2 00/17] locks: internal lease API overhaul Jeff Layton
2014-09-04 12:38 ` [PATCH v2 01/17] locks: consolidate "nolease" routines Jeff Layton
2014-09-04 12:38   ` [Cluster-devel] " Jeff Layton
2014-09-04 12:38   ` Jeff Layton
2014-09-04 12:41   ` Trond Myklebust
2014-09-04 12:41     ` [Cluster-devel] " Trond Myklebust
2014-09-04 12:41     ` Trond Myklebust
2014-09-04 12:49     ` Jeff Layton
2014-09-04 12:49       ` [Cluster-devel] " Jeff Layton
2014-09-04 12:49       ` Jeff Layton
2014-09-04 18:25       ` Trond Myklebust
2014-09-04 18:25         ` [Cluster-devel] " Trond Myklebust
2014-09-04 20:12         ` Christoph Hellwig
2014-09-04 20:12           ` [Cluster-devel] " Christoph Hellwig
2014-09-05 11:48           ` Jeff Layton
2014-09-05 11:48             ` [Cluster-devel] " Jeff Layton
2014-09-05 11:48             ` Jeff Layton
2014-09-04 17:46   ` Christoph Hellwig
2014-09-04 17:46     ` [Cluster-devel] " Christoph Hellwig
2014-09-04 17:46     ` Christoph Hellwig
2014-09-04 12:38 ` [PATCH v2 02/17] security: make security_file_set_fowner, f_setown and __f_setown void return Jeff Layton
2014-09-04 17:47   ` Christoph Hellwig
2014-10-07 17:11   ` Dmitry Kasatkin
2014-10-07 17:17     ` Christoph Hellwig
2014-10-07 17:34       ` Dmitry Kasatkin
2014-10-07 17:34         ` Dmitry Kasatkin
2014-10-07 18:02         ` Jeff Layton
2014-09-04 12:38 ` [PATCH v2 03/17] locks: close potential race in lease_get_mtime Jeff Layton
2014-09-04 12:38 ` [PATCH v2 04/17] nfsd: fix potential lease memory leak in nfs4_setlease Jeff Layton
2014-09-04 12:38 ` Jeff Layton [this message]
2014-09-04 20:14   ` [PATCH v2 05/17] locks: generic_delete_lease doesn't need a file_lock at all Christoph Hellwig
2014-09-04 20:14     ` Christoph Hellwig
2014-09-05  0:29     ` Jeff Layton
2014-09-05  0:29       ` Jeff Layton
2015-01-12 23:03   ` NeilBrown
2015-01-12 23:03     ` NeilBrown
2015-01-12 23:25     ` Jeff Layton
2015-01-13  2:14       ` NeilBrown
2015-01-13  2:14         ` NeilBrown
2014-09-04 12:38 ` [PATCH v2 06/17] locks: clean up vfs_setlease kerneldoc comments Jeff Layton
2014-09-04 12:38 ` [PATCH v2 07/17] nfsd: don't keep a pointer to the lease in nfs4_file Jeff Layton
2014-09-05 21:40   ` J. Bruce Fields
2014-09-05 21:40     ` J. Bruce Fields
2014-09-06 12:33     ` Jeff Layton
2014-09-06 12:33       ` Jeff Layton
2014-09-04 12:38 ` [PATCH v2 08/17] locks: plumb a "priv" pointer into the setlease routines Jeff Layton
2014-09-04 17:48   ` Christoph Hellwig
2014-09-04 12:38 ` [PATCH v2 09/17] locks: define a lm_setup handler for leases Jeff Layton
2014-09-04 12:38   ` Jeff Layton
2014-09-04 17:49   ` Christoph Hellwig
2014-09-04 17:49     ` Christoph Hellwig
2014-09-04 12:38 ` [PATCH v2 10/17] locks: move i_lock acquisition into generic_*_lease handlers Jeff Layton
2014-09-04 12:38 ` [PATCH v2 11/17] locks: move freeing of leases outside of i_lock Jeff Layton
2014-09-04 17:50   ` Christoph Hellwig
2014-09-04 17:50     ` Christoph Hellwig
2014-09-05 14:03     ` Jeff Layton
2014-09-04 12:38 ` [PATCH v2 12/17] locks: update Documentation/filesystems with new setlease semantics Jeff Layton
2014-09-04 17:50   ` Christoph Hellwig
2014-09-04 17:50     ` Christoph Hellwig
2014-09-05 14:02     ` Jeff Layton
2014-09-05 14:02       ` Jeff Layton
2014-09-04 12:38 ` [PATCH v2 13/17] locks: remove i_have_this_lease check from __break_lease Jeff Layton
2014-09-04 12:38   ` Jeff Layton
2014-09-04 17:51   ` Christoph Hellwig
2014-09-04 17:51     ` Christoph Hellwig
2014-09-04 18:03     ` Jeff Layton
2014-09-04 18:03       ` Jeff Layton
2014-09-04 12:38 ` [PATCH v2 14/17] locks: __break_lease cleanup in preparation of allowing direct removal of leases Jeff Layton
2014-09-04 18:07   ` Christoph Hellwig
2014-09-04 18:07     ` Christoph Hellwig
2014-09-05 13:35     ` Jeff Layton
2014-09-05 13:35       ` Jeff Layton
2014-09-04 12:38 ` [PATCH v2 15/17] locks: give lm_break a return value Jeff Layton
2014-09-04 12:38   ` Jeff Layton
2014-09-04 18:08   ` Christoph Hellwig
2014-09-04 18:08     ` Christoph Hellwig
2014-09-04 12:38 ` [PATCH v2 16/17] locks: set fl_owner for leases to filp instead of current->files Jeff Layton
2014-09-04 12:38   ` Jeff Layton
2014-09-04 12:38 ` [PATCH v2 17/17] locks: clean up comments over fl_owner_t definition Jeff Layton
2014-09-04 17:53   ` Christoph Hellwig
2014-09-04 17:53     ` Christoph Hellwig
2014-09-05 13:36     ` Jeff Layton
2014-09-05 13:36       ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1409834323-7171-6-git-send-email-jlayton@primarydata.com \
    --to=jlayton@primarydata.com \
    --cc=bfields@fieldses.org \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.