Re: [PATCH 02/13] filelock: add a lm_may_setlease lease_manager callback

On Tue, 14 Oct 2025, Jeff Layton wrote:
> On Tue, 2025-10-14 at 16:34 +1100, NeilBrown wrote:
> > On Tue, 14 Oct 2025, Jeff Layton wrote:
> > > The NFSv4.1 protocol adds support for directory delegations, but it
> > > specifies that if you already have a delegation and try to request a new
> > > one on the same filehandle, the server must reply that the delegation is
> > > unavailable.
> > > 
> > > Add a new lease manager callback to allow the lease manager (nfsd in
> > > this case) to impose this extra check when performing a setlease.
> > > 
> > > Signed-off-by: Jeff Layton <[email protected]>
> > > ---
> > >  fs/locks.c               |  5 +++++
> > >  include/linux/filelock.h | 14 ++++++++++++++
> > >  2 files changed, 19 insertions(+)
> > > 
> > > diff --git a/fs/locks.c b/fs/locks.c
> > > index 0b16921fb52e602ea2e0c3de39d9d772af98ba7d..9e366b13674538dbf482ffdeee92fc717733ee20 100644
> > > --- a/fs/locks.c
> > > +++ b/fs/locks.c
> > > @@ -1826,6 +1826,11 @@ generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **pr
> > >  			continue;
> > >  		}
> > >  
> > > +		/* Allow the lease manager to veto the setlease */
> > > +		if (lease->fl_lmops->lm_may_setlease &&
> > > +		    !lease->fl_lmops->lm_may_setlease(lease, fl))
> > > +			goto out;
> > > +
> > 
> > I don't see any locking around this.  What if the condition which
> > triggers a veto happens after this check, and before the lm_change
> > below?
> > Should lm_change implement the veto?  Return -EAGAIN?
> > 
> > 
> 
> The flc_lock is held over this check and any subsequent lease addition.
> Is that not sufficient?

Ah - I didn't see that - sorry.

But I still wonder why ->lm_change cannot do the veto.

I also wonder if the current code can work.  If that loop finds an
existing lease with the same file and the same owner the it invokes
"continue" before the code that you added.
So unless I'm misunderstanding (again) in the case that you are
interested in, the new code doesn't run.

Thanks,
NeilBrown