Re: [PATCH 1/2] filelock: add lease_dispose_list() helper

[Chuck Lever <cel@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