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

J. Bruce Fields
2010-11-04T01:41:34Z

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