locks: give lm_break a return value

Christoph suggests:

   "Add a return value to lm_break so that the lock manager can tell the
    core code "you can delete this lease right now".  That gets rid of
    the games with the timeout which require all kinds of race avoidance
    code in the users."

Do that here and have the nfsd lease break routine use it when it detects
that there was a race between setting up the lease and it being broken.

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 7d627ac0ed8724f934fb3df9691e2b03b5a2752d..aed4a957d232df199b73557185ed571cfbcf0ef5 100644 (file)
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -427,9 +427,11 @@ static int flock_to_posix_lock(struct file *filp, struct file_lock *fl,
 }
 
 /* default lease lock manager operations */
-static void lease_break_callback(struct file_lock *fl)
+static bool
+lease_break_callback(struct file_lock *fl)
 {
        kill_fasync(&fl->fl_fasync, SIGIO, POLL_MSG);
+       return false;
 }
 
 static void
@@ -1382,7 +1384,7 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
 {
        int error = 0;
        struct file_lock *new_fl;
-       struct file_lock *fl;
+       struct file_lock *fl, **before;
        unsigned long break_time;
        int want_write = (mode & O_ACCMODE) != O_RDONLY;
        LIST_HEAD(dispose);
@@ -1406,7 +1408,9 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
                        break_time++;   /* so that 0 means no break time */
        }
 
-       for (fl = inode->i_flock; fl && IS_LEASE(fl); fl = fl->fl_next) {
+       for (before = &inode->i_flock;
+                       ((fl = *before) != NULL) && IS_LEASE(fl);
+                       before = &fl->fl_next) {
                if (!leases_conflict(fl, new_fl))
                        continue;
                if (want_write) {
@@ -1420,9 +1424,14 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
                        fl->fl_flags |= FL_DOWNGRADE_PENDING;
                        fl->fl_downgrade_time = break_time;
                }
-               fl->fl_lmops->lm_break(fl);
+               if (fl->fl_lmops->lm_break(fl))
+                       locks_delete_lock(before, &dispose);
        }
 
+       fl = inode->i_flock;
+       if (!fl || !IS_LEASE(fl))
+               goto out;
+
        if (mode & O_NONBLOCK) {
                trace_break_lease_noblock(inode, new_fl);
                error = -EWOULDBLOCK;

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 604ab6decd28a7684012ffa891db97fac7e37e5b..d1b851548b7afc77fbfb75a523a21fbf558698a5 100644 (file)
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3391,18 +3391,20 @@ static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
 }
 
 /* Called from break_lease() with i_lock held. */
-static void nfsd_break_deleg_cb(struct file_lock *fl)
+static bool
+nfsd_break_deleg_cb(struct file_lock *fl)
 {
+       bool ret = false;
        struct nfs4_file *fp = (struct nfs4_file *)fl->fl_owner;
        struct nfs4_delegation *dp;
 
        if (!fp) {
                WARN(1, "(%p)->fl_owner NULL\n", fl);
-               return;
+               return ret;
        }
        if (fp->fi_had_conflict) {
                WARN(1, "duplicate break on %p\n", fp);
-               return;
+               return ret;
        }
        /*
         * We don't want the locks code to timeout the lease for us;
@@ -3414,17 +3416,16 @@ static void nfsd_break_deleg_cb(struct file_lock *fl)
        spin_lock(&fp->fi_lock);
        fp->fi_had_conflict = true;
        /*
-        * If there are no delegations on the list, then we can't count on this
-        * lease ever being cleaned up. Set the fl_break_time to jiffies so that
-        * time_out_leases will do it ASAP. The fact that fi_had_conflict is now
-        * true should keep any new delegations from being hashed.
+        * If there are no delegations on the list, then return true
+        * so that the lease code will go ahead and delete it.
         */
        if (list_empty(&fp->fi_delegations))
-               fl->fl_break_time = jiffies;
+               ret = true;
        else
                list_for_each_entry(dp, &fp->fi_delegations, dl_perfile)
                        nfsd_break_one_deleg(dp);
        spin_unlock(&fp->fi_lock);
+       return ret;
 }
 
 static int

diff --git a/include/linux/fs.h b/include/linux/fs.h
index f419f718e447e365893f4d9ccd8a03cb8b16d41a..ed4e1897099c6936b9dd0229f52a6ba8e56fbcca 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -872,7 +872,7 @@ struct lock_manager_operations {
        void (*lm_put_owner)(struct file_lock *);
        void (*lm_notify)(struct file_lock *);  /* unblock callback */
        int (*lm_grant)(struct file_lock *, int);
-       void (*lm_break)(struct file_lock *);
+       bool (*lm_break)(struct file_lock *);
        int (*lm_change)(struct file_lock **, int, struct list_head *);
        void (*lm_setup)(struct file_lock *, void **);
 };