linux-kernel - Re: [PATCH 1/2] filelock: add lease_dispose

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a5da76b6-6b84-42b7-a9c5-8f271a546af1@kernel.org>
Date: Wed, 3 Dec 2025 14:35:00 -0500
From: Chuck Lever <cel@...nel.org>
To: Jeff Layton <jlayton@...nel.org>, Alexander Viro
 <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>,
 Jan Kara <jack@...e.cz>, Chuck Lever <chuck.lever@...cle.com>,
 Alexander Aring <alex.aring@...il.com>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>,
 Jonathan Corbet <corbet@....net>, NeilBrown <neil@...wn.name>,
 Olga Kornievskaia <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>,
 Tom Talpey <tom@...pey.com>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-nfs@...r.kernel.org
Subject: Re: [PATCH 1/2] filelock: add lease_dispose_list() helper

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@...nel.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