...and call that from the lease handling code instead of
locks_dispose_list(). Remove the lease handling parts from
locks_dispose_list().

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/locks.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 7f4ccc7974bc8d3e82500ee692c6520b53f2280f..e974f8e180fe48682a271af4f143e6bc8e9c4d3b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -369,10 +369,19 @@ locks_dispose_list(struct list_head *dispose)
 	while (!list_empty(dispose)) {
 		flc = list_first_entry(dispose, struct file_lock_core, flc_list);
 		list_del_init(&flc->flc_list);
-		if (flc->flc_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))
-			locks_free_lease(file_lease(flc));
-		else
-			locks_free_lock(file_lock(flc));
+		locks_free_lock(file_lock(flc));
+	}
+}
+
+static void
+lease_dispose_list(struct list_head *dispose)
+{
+	struct file_lock_core *flc;
+
+	while (!list_empty(dispose)) {
+		flc = list_first_entry(dispose, struct file_lock_core, flc_list);
+		list_del_init(&flc->flc_list);
+		locks_free_lease(file_lease(flc));
 	}
 }
 
@@ -1620,7 +1629,7 @@ int __break_lease(struct inode *inode, unsigned int flags)
 	spin_unlock(&ctx->flc_lock);
 	percpu_up_read(&file_rwsem);
 
-	locks_dispose_list(&dispose);
+	lease_dispose_list(&dispose);
 	error = wait_event_interruptible_timeout(new_fl->c.flc_wait,
 						 list_empty(&new_fl->c.flc_blocked_member),
 						 break_time);
@@ -1643,7 +1652,7 @@ int __break_lease(struct inode *inode, unsigned int flags)
 out:
 	spin_unlock(&ctx->flc_lock);
 	percpu_up_read(&file_rwsem);
-	locks_dispose_list(&dispose);
+	lease_dispose_list(&dispose);
 free_lock:
 	locks_free_lease(new_fl);
 	return error;
@@ -1726,7 +1735,7 @@ static int __fcntl_getlease(struct file *filp, unsigned int flavor)
 		spin_unlock(&ctx->flc_lock);
 		percpu_up_read(&file_rwsem);
 
-		locks_dispose_list(&dispose);
+		lease_dispose_list(&dispose);
 	}
 	return type;
 }
@@ -1895,7 +1904,7 @@ generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **pr
 out:
 	spin_unlock(&ctx->flc_lock);
 	percpu_up_read(&file_rwsem);
-	locks_dispose_list(&dispose);
+	lease_dispose_list(&dispose);
 	if (is_deleg)
 		inode_unlock(inode);
 	if (!error && !my_fl)
@@ -1931,7 +1940,7 @@ static int generic_delete_lease(struct file *filp, void *owner)
 		error = fl->fl_lmops->lm_change(victim, F_UNLCK, &dispose);
 	spin_unlock(&ctx->flc_lock);
 	percpu_up_read(&file_rwsem);
-	locks_dispose_list(&dispose);
+	lease_dispose_list(&dispose);
 	return error;
 }
 
@@ -2726,7 +2735,7 @@ locks_remove_lease(struct file *filp, struct file_lock_context *ctx)
 	spin_unlock(&ctx->flc_lock);
 	percpu_up_read(&file_rwsem);
 
-	locks_dispose_list(&dispose);
+	lease_dispose_list(&dispose);
 }
 
 /*

-- 
2.52.0

On Mon, Dec 1, 2025, at 10:08 AM, Jeff Layton wrote:
> ...and call that from the lease handling code instead of
> locks_dispose_list(). Remove the lease handling parts from
> locks_dispose_list().

The actual change here isn't bothering me, but I'm having trouble
understanding why it's needed. It doesn't appear to be a strict
functional prerequisite for 2/2.

A little more context in the commit message would be helpful.
Sample commit description:

  The lease-handling code paths always know they're disposing of leases,
  yet locks_dispose_list() checks flags at runtime to determine whether
  to call locks_free_lease() or locks_free_lock().

  Split out a dedicated lease_dispose_list() helper for lease code paths.
  This makes the type handling explicit and prepares for the upcoming
  lease_manager enhancements where lease-specific operations are being
  consolidated.

But that reflects only my naive understanding of the patch. You
might have something else in mind.


> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/locks.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 
> 7f4ccc7974bc8d3e82500ee692c6520b53f2280f..e974f8e180fe48682a271af4f143e6bc8e9c4d3b 
> 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -369,10 +369,19 @@ locks_dispose_list(struct list_head *dispose)
>  	while (!list_empty(dispose)) {
>  		flc = list_first_entry(dispose, struct file_lock_core, flc_list);
>  		list_del_init(&flc->flc_list);
> -		if (flc->flc_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))
> -			locks_free_lease(file_lease(flc));
> -		else
> -			locks_free_lock(file_lock(flc));
> +		locks_free_lock(file_lock(flc));
> +	}
> +}
> +
> +static void
> +lease_dispose_list(struct list_head *dispose)
> +{
> +	struct file_lock_core *flc;
> +
> +	while (!list_empty(dispose)) {
> +		flc = list_first_entry(dispose, struct file_lock_core, flc_list);
> +		list_del_init(&flc->flc_list);
> +		locks_free_lease(file_lease(flc));
>  	}
>  }
> 
> @@ -1620,7 +1629,7 @@ int __break_lease(struct inode *inode, unsigned int flags)
>  	spin_unlock(&ctx->flc_lock);
>  	percpu_up_read(&file_rwsem);
> 
> -	locks_dispose_list(&dispose);
> +	lease_dispose_list(&dispose);
>  	error = wait_event_interruptible_timeout(new_fl->c.flc_wait,
>  						 list_empty(&new_fl->c.flc_blocked_member),
>  						 break_time);
> @@ -1643,7 +1652,7 @@ int __break_lease(struct inode *inode, unsigned 
> int flags)
>  out:
>  	spin_unlock(&ctx->flc_lock);
>  	percpu_up_read(&file_rwsem);
> -	locks_dispose_list(&dispose);
> +	lease_dispose_list(&dispose);
>  free_lock:
>  	locks_free_lease(new_fl);
>  	return error;
> @@ -1726,7 +1735,7 @@ static int __fcntl_getlease(struct file *filp, 
> unsigned int flavor)
>  		spin_unlock(&ctx->flc_lock);
>  		percpu_up_read(&file_rwsem);
> 
> -		locks_dispose_list(&dispose);
> +		lease_dispose_list(&dispose);
>  	}
>  	return type;
>  }
> @@ -1895,7 +1904,7 @@ generic_add_lease(struct file *filp, int arg, 
> struct file_lease **flp, void **pr
>  out:
>  	spin_unlock(&ctx->flc_lock);
>  	percpu_up_read(&file_rwsem);
> -	locks_dispose_list(&dispose);
> +	lease_dispose_list(&dispose);
>  	if (is_deleg)
>  		inode_unlock(inode);
>  	if (!error && !my_fl)
> @@ -1931,7 +1940,7 @@ static int generic_delete_lease(struct file 
> *filp, void *owner)
>  		error = fl->fl_lmops->lm_change(victim, F_UNLCK, &dispose);
>  	spin_unlock(&ctx->flc_lock);
>  	percpu_up_read(&file_rwsem);
> -	locks_dispose_list(&dispose);
> +	lease_dispose_list(&dispose);
>  	return error;
>  }
> 
> @@ -2726,7 +2735,7 @@ locks_remove_lease(struct file *filp, struct 
> file_lock_context *ctx)
>  	spin_unlock(&ctx->flc_lock);
>  	percpu_up_read(&file_rwsem);
> 
> -	locks_dispose_list(&dispose);
> +	lease_dispose_list(&dispose);
>  }
> 
>  /*
>
> -- 
> 2.52.0

-- 
Chuck Lever

On Thu, 04 Dec 2025, Chuck Lever wrote:
> 
> On Mon, Dec 1, 2025, at 10:08 AM, Jeff Layton wrote:
> > ...and call that from the lease handling code instead of
> > locks_dispose_list(). Remove the lease handling parts from
> > locks_dispose_list().
> 
> The actual change here isn't bothering me, but I'm having trouble
> understanding why it's needed. It doesn't appear to be a strict
> functional prerequisite for 2/2.

This was almost exactly my thought too.  The commit message should say
*why* the change is being made and this one just left us guessing.
But I *do* like the change and would rather it were kept in the series,
but with a simple addition to the commit message saying that is a
simplification that isn't strictly necessary.

Thanks,
NeilBrown

On Wed, 2025-12-03 at 13:55 -0500, Chuck Lever wrote:
> 
> On Mon, Dec 1, 2025, at 10:08 AM, Jeff Layton wrote:
> > ...and call that from the lease handling code instead of
> > locks_dispose_list(). Remove the lease handling parts from
> > locks_dispose_list().
> 
> The actual change here isn't bothering me, but I'm having trouble
> understanding why it's needed. It doesn't appear to be a strict
> functional prerequisite for 2/2.
> 

It's not. We can table this patch for now if that's preferable, but I
do think it's a worthwhile cleanup.
 
> A little more context in the commit message would be helpful.
> Sample commit description:
> 
>   The lease-handling code paths always know they're disposing of leases,
>   yet locks_dispose_list() checks flags at runtime to determine whether
>   to call locks_free_lease() or locks_free_lock().
> 
>   Split out a dedicated lease_dispose_list() helper for lease code paths.
>   This makes the type handling explicit and prepares for the upcoming
>   lease_manager enhancements where lease-specific operations are being
>   consolidated.
> 

I may crib this if I end up resending it.

> But that reflects only my naive understanding of the patch. You
> might have something else in mind.
> 
> 

Nope, no ulterior motive here. It's just a nice to have cleanup that
helps to further separate the lock and lease handling code.

> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/locks.c | 29 +++++++++++++++++++----------
> >  1 file changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git a/fs/locks.c b/fs/locks.c
> > index 
> > 7f4ccc7974bc8d3e82500ee692c6520b53f2280f..e974f8e180fe48682a271af4f143e6bc8e9c4d3b 
> > 100644
> > --- a/fs/locks.c
> > +++ b/fs/locks.c
> > @@ -369,10 +369,19 @@ locks_dispose_list(struct list_head *dispose)
> >  	while (!list_empty(dispose)) {
> >  		flc = list_first_entry(dispose, struct file_lock_core, flc_list);
> >  		list_del_init(&flc->flc_list);
> > -		if (flc->flc_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))
> > -			locks_free_lease(file_lease(flc));
> > -		else
> > -			locks_free_lock(file_lock(flc));
> > +		locks_free_lock(file_lock(flc));
> > +	}
> > +}
> > +
> > +static void
> > +lease_dispose_list(struct list_head *dispose)
> > +{
> > +	struct file_lock_core *flc;
> > +
> > +	while (!list_empty(dispose)) {
> > +		flc = list_first_entry(dispose, struct file_lock_core, flc_list);
> > +		list_del_init(&flc->flc_list);
> > +		locks_free_lease(file_lease(flc));
> >  	}
> >  }
> > 
> > @@ -1620,7 +1629,7 @@ int __break_lease(struct inode *inode, unsigned int flags)
> >  	spin_unlock(&ctx->flc_lock);
> >  	percpu_up_read(&file_rwsem);
> > 
> > -	locks_dispose_list(&dispose);
> > +	lease_dispose_list(&dispose);
> >  	error = wait_event_interruptible_timeout(new_fl->c.flc_wait,
> >  						 list_empty(&new_fl->c.flc_blocked_member),
> >  						 break_time);
> > @@ -1643,7 +1652,7 @@ int __break_lease(struct inode *inode, unsigned 
> > int flags)
> >  out:
> >  	spin_unlock(&ctx->flc_lock);
> >  	percpu_up_read(&file_rwsem);
> > -	locks_dispose_list(&dispose);
> > +	lease_dispose_list(&dispose);
> >  free_lock:
> >  	locks_free_lease(new_fl);
> >  	return error;
> > @@ -1726,7 +1735,7 @@ static int __fcntl_getlease(struct file *filp, 
> > unsigned int flavor)
> >  		spin_unlock(&ctx->flc_lock);
> >  		percpu_up_read(&file_rwsem);
> > 
> > -		locks_dispose_list(&dispose);
> > +		lease_dispose_list(&dispose);
> >  	}
> >  	return type;
> >  }
> > @@ -1895,7 +1904,7 @@ generic_add_lease(struct file *filp, int arg, 
> > struct file_lease **flp, void **pr
> >  out:
> >  	spin_unlock(&ctx->flc_lock);
> >  	percpu_up_read(&file_rwsem);
> > -	locks_dispose_list(&dispose);
> > +	lease_dispose_list(&dispose);
> >  	if (is_deleg)
> >  		inode_unlock(inode);
> >  	if (!error && !my_fl)
> > @@ -1931,7 +1940,7 @@ static int generic_delete_lease(struct file 
> > *filp, void *owner)
> >  		error = fl->fl_lmops->lm_change(victim, F_UNLCK, &dispose);
> >  	spin_unlock(&ctx->flc_lock);
> >  	percpu_up_read(&file_rwsem);
> > -	locks_dispose_list(&dispose);
> > +	lease_dispose_list(&dispose);
> >  	return error;
> >  }
> > 
> > @@ -2726,7 +2735,7 @@ locks_remove_lease(struct file *filp, struct 
> > file_lock_context *ctx)
> >  	spin_unlock(&ctx->flc_lock);
> >  	percpu_up_read(&file_rwsem);
> > 
> > -	locks_dispose_list(&dispose);
> > +	lease_dispose_list(&dispose);
> >  }
> > 
> >  /*
> > 
> > -- 
> > 2.52.0

-- 
Jeff Layton <jlayton@kernel.org>

On 12/3/25 2:33 PM, Jeff Layton wrote:
> On Wed, 2025-12-03 at 13:55 -0500, Chuck Lever wrote:
>>
>> On Mon, Dec 1, 2025, at 10:08 AM, Jeff Layton wrote:
>>> ...and call that from the lease handling code instead of
>>> locks_dispose_list(). Remove the lease handling parts from
>>> locks_dispose_list().
>>
>> The actual change here isn't bothering me, but I'm having trouble
>> understanding why it's needed. It doesn't appear to be a strict
>> functional prerequisite for 2/2.
>>
> 
> It's not. We can table this patch for now if that's preferable, but I
> do think it's a worthwhile cleanup.
>  
>> A little more context in the commit message would be helpful.
>> Sample commit description:
>>
>>   The lease-handling code paths always know they're disposing of leases,
>>   yet locks_dispose_list() checks flags at runtime to determine whether
>>   to call locks_free_lease() or locks_free_lock().
>>
>>   Split out a dedicated lease_dispose_list() helper for lease code paths.
>>   This makes the type handling explicit and prepares for the upcoming
>>   lease_manager enhancements where lease-specific operations are being
>>   consolidated.
>>
> 
> I may crib this if I end up resending it.
> 
>> But that reflects only my naive understanding of the patch. You
>> might have something else in mind.
>>
>>
> 
> Nope, no ulterior motive here. It's just a nice to have cleanup that
> helps to further separate the lock and lease handling code.

Yep, it's a good clean-up.


>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>> ---
>>>  fs/locks.c | 29 +++++++++++++++++++----------
>>>  1 file changed, 19 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/fs/locks.c b/fs/locks.c
>>> index 
>>> 7f4ccc7974bc8d3e82500ee692c6520b53f2280f..e974f8e180fe48682a271af4f143e6bc8e9c4d3b 
>>> 100644
>>> --- a/fs/locks.c
>>> +++ b/fs/locks.c
>>> @@ -369,10 +369,19 @@ locks_dispose_list(struct list_head *dispose)
>>>  	while (!list_empty(dispose)) {
>>>  		flc = list_first_entry(dispose, struct file_lock_core, flc_list);
>>>  		list_del_init(&flc->flc_list);
>>> -		if (flc->flc_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))
>>> -			locks_free_lease(file_lease(flc));
>>> -		else
>>> -			locks_free_lock(file_lock(flc));
>>> +		locks_free_lock(file_lock(flc));
>>> +	}
>>> +}
>>> +
>>> +static void
>>> +lease_dispose_list(struct list_head *dispose)
>>> +{
>>> +	struct file_lock_core *flc;
>>> +
>>> +	while (!list_empty(dispose)) {
>>> +		flc = list_first_entry(dispose, struct file_lock_core, flc_list);
>>> +		list_del_init(&flc->flc_list);
>>> +		locks_free_lease(file_lease(flc));
>>>  	}
>>>  }
>>>
>>> @@ -1620,7 +1629,7 @@ int __break_lease(struct inode *inode, unsigned int flags)
>>>  	spin_unlock(&ctx->flc_lock);
>>>  	percpu_up_read(&file_rwsem);
>>>
>>> -	locks_dispose_list(&dispose);
>>> +	lease_dispose_list(&dispose);
>>>  	error = wait_event_interruptible_timeout(new_fl->c.flc_wait,
>>>  						 list_empty(&new_fl->c.flc_blocked_member),
>>>  						 break_time);
>>> @@ -1643,7 +1652,7 @@ int __break_lease(struct inode *inode, unsigned 
>>> int flags)
>>>  out:
>>>  	spin_unlock(&ctx->flc_lock);
>>>  	percpu_up_read(&file_rwsem);
>>> -	locks_dispose_list(&dispose);
>>> +	lease_dispose_list(&dispose);
>>>  free_lock:
>>>  	locks_free_lease(new_fl);
>>>  	return error;
>>> @@ -1726,7 +1735,7 @@ static int __fcntl_getlease(struct file *filp, 
>>> unsigned int flavor)
>>>  		spin_unlock(&ctx->flc_lock);
>>>  		percpu_up_read(&file_rwsem);
>>>
>>> -		locks_dispose_list(&dispose);
>>> +		lease_dispose_list(&dispose);
>>>  	}
>>>  	return type;
>>>  }
>>> @@ -1895,7 +1904,7 @@ generic_add_lease(struct file *filp, int arg, 
>>> struct file_lease **flp, void **pr
>>>  out:
>>>  	spin_unlock(&ctx->flc_lock);
>>>  	percpu_up_read(&file_rwsem);
>>> -	locks_dispose_list(&dispose);
>>> +	lease_dispose_list(&dispose);
>>>  	if (is_deleg)
>>>  		inode_unlock(inode);
>>>  	if (!error && !my_fl)
>>> @@ -1931,7 +1940,7 @@ static int generic_delete_lease(struct file 
>>> *filp, void *owner)
>>>  		error = fl->fl_lmops->lm_change(victim, F_UNLCK, &dispose);
>>>  	spin_unlock(&ctx->flc_lock);
>>>  	percpu_up_read(&file_rwsem);
>>> -	locks_dispose_list(&dispose);
>>> +	lease_dispose_list(&dispose);
>>>  	return error;
>>>  }
>>>
>>> @@ -2726,7 +2735,7 @@ locks_remove_lease(struct file *filp, struct 
>>> file_lock_context *ctx)
>>>  	spin_unlock(&ctx->flc_lock);
>>>  	percpu_up_read(&file_rwsem);
>>>
>>> -	locks_dispose_list(&dispose);
>>> +	lease_dispose_list(&dispose);
>>>  }
>>>
>>>  /*
>>>
>>> -- 
>>> 2.52.0
> 


-- 
Chuck Lever