locks: generic_delete_lease doesn't need a file_lock at all

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>

diff --git a/fs/locks.c b/fs/locks.c
index def1ac2e87bdce62e4817c18825e68a5fe761c09..f79c74ef51efaf04b10b07e02a368c23d9991c34 100644 (file)
--- 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 5bb4952faf5b4b5e7025c0d608bcf3f9f7cab09e..89d54e505155958e31466d6f4c938c12f0acb7d8 100644 (file)
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -690,7 +690,7 @@ static void nfs4_put_deleg_lease(struct nfs4_file *fp)
        spin_unlock(&fp->fi_lock);
 
        if (filp) {
-               vfs_setlease(filp, F_UNLCK, &fl);
+               vfs_setlease(filp, F_UNLCK, NULL);
                fput(filp);
        }
 }

diff --git a/include/trace/events/filelock.h b/include/trace/events/filelock.h
index 59d11c22f07639ca586a46644ef32b6e061b4ce1..a0d008070962b6b153b979e51108e3d23b4a5935 100644 (file)
--- 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",