Re: [PATCH 1/2] locks: let the caller free file_lock on ->setlease failure

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, Nov 03, 2010 at 09:40:24PM -0400, J. Bruce Fields wrote:
> On Wed, Nov 03, 2010 at 04:41:48PM -0400, J. Bruce Fields wrote:
> > On Sun, Oct 31, 2010 at 08:35:10AM -0400, Christoph Hellwig wrote:
> > > Index: linux-2.6/fs/locks.c
> > > ===================================================================
> > > --- linux-2.6.orig/fs/locks.c	2010-10-31 07:10:07.649004084 -0400
> > > +++ linux-2.6/fs/locks.c	2010-10-31 07:34:10.102255587 -0400
> > > @@ -1428,8 +1425,9 @@ int generic_setlease(struct file *filp,
> > >  		goto out;
> > >  
> > >  	if (my_before != NULL) {
> > > -		*flp = *my_before;
> > >  		error = lease->fl_lmops->fl_change(my_before, arg);
> > > +		if (!error)
> > > +			*flp = *my_before;
> > 
> > Argh, missed this: we're leaking the passed-in lease in this case.
> 
> We could do something like this.

And while I was here I noticed:

--b.

>From 072be1f975c6f839661cfc4f6c45ccb944417eea Mon Sep 17 00:00:00 2001
From: J. Bruce Fields <bfields@redhat.com>
Date: Wed, 3 Nov 2010 18:09:18 -0400
Subject: [PATCH 2/2] locks: remove dead lease error-handling code

A minor oversight from f7347ce4ee7c65415f84be915c018473e7076f31,
"fasync: re-organize fasync entry insertion to allow it under a
spinlock": this cleanup-on-error was only needed to handle -ENOMEM.  Now
that we're preallocating it's unneeded.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/locks.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 61c22f7..0e62dd3 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1506,7 +1506,6 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
 {
 	struct file_lock *fl, *ret;
 	struct fasync_struct *new;
-	struct inode *inode = filp->f_path.dentry->d_inode;
 	int error;
 
 	fl = lease_alloc(filp, arg);
@@ -1520,7 +1519,7 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
 	}
 	ret = fl;
 	lock_flocks();
-	error = __vfs_setlease(filp, arg, &fl);
+	error = __vfs_setlease(filp, arg, &ret);
 	if (error) {
 		unlock_flocks();
 		locks_free_lock(fl);
@@ -1538,14 +1537,7 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
 	if (!fasync_insert_entry(fd, filp, &ret->fl_fasync, new))
 		new = NULL;
 
-	if (error < 0) {
-		/* remove lease just inserted by setlease */
-		fl->fl_type = F_UNLCK | F_INPROGRESS;
-		fl->fl_break_time = jiffies - 10;
-		time_out_leases(inode);
-	} else {
-		error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
-	}
+	error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
 	unlock_flocks();
 
 out_free_fasync:
-- 
1.7.1