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

[J. Bruce Fields]

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.

The irritating thing is that the only lease user I understand is the
nfsd code, and it doesn't want this lease-merging behavior; the only
reason that fl_change is there is so it can just turn this case into an
error every time.

And I have no idea what the requirements are of any other users: do
leases behave like this on purpose, or was it just an arbitrary choice,
and does anyone depend on it now?

In the end maybe it would be better just to leave leases as they are and
define a new lock type for nfsd.

We'd probably have to do that eventually anyway, and it'd save me trying
to guess what the lease semantics are supposed to be....

--b.