Re: [PATCH 2/4] leases: when unlocking, skip locking-related steps

["J. Bruce Fields" <bfields@fieldses.org>]

On Thu, Apr 24, 2008 at 10:32:39AM -0400, Chuck Lever wrote:
> Hi David-
>
> On Apr 23, 2008, at 4:29 PM, David M. Richter wrote:
>> In generic_setlease(), we don't need to allocate a new struct  
>> file_lock
>> or check for readers or writers when called with F_UNLCK.
>
> Since you re-ordered the locks_alloc_lock call in the next patch, it  
> seems to me this would look a lot cleaner with a switch statement  
> instead of all these "if (arg == F_FOO)".

This could certainly be cleaner, but I took a quick look and didn't see
how to do what you were suggesting--probably just me being dense.

David's changes seem reasonable enough on their own, so I'd take any
cleanup as an incremental patch on top.

--b.

>
>> Signed-off-by: David M. Richter <richterd@citi.umich.edu>
>> ---
>> fs/locks.c |   24 +++++++++++++-----------
>> 1 files changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/locks.c b/fs/locks.c
>> index b9f3a0b..da1d0dd 100644
>> --- a/fs/locks.c
>> +++ b/fs/locks.c
>> @@ -1367,18 +1367,20 @@ int generic_setlease(struct file *filp, long  
>> arg, struct file_lock **flp)
>>
>> 	lease = *flp;
>>
>> -	error = -EAGAIN;
>> -	if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
>> -		goto out;
>> -	if ((arg == F_WRLCK)
>> -	    && ((atomic_read(&dentry->d_count) > 1)
>> -		|| (atomic_read(&inode->i_count) > 1)))
>> -		goto out;
>> +	if (arg != F_UNLCK) {
>> +		error = -EAGAIN;
>> +		if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
>> +			goto out;
>> +		if ((arg == F_WRLCK)
>> +		    && ((atomic_read(&dentry->d_count) > 1)
>> +			|| (atomic_read(&inode->i_count) > 1)))
>> +			goto out;
>>
>> -	error = -ENOMEM;
>> -	new_fl = locks_alloc_lock();
>> -	if (new_fl == NULL)
>> -		goto out;
>> +		error = -ENOMEM;
>> +		new_fl = locks_alloc_lock();
>> +		if (new_fl == NULL)
>> +			goto out;
>> +	}
>>
>> 	/*
>> 	 * At this point, we know that if there is an exclusive
>> -- 
>> 1.5.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux- 
>> fsdevel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>