[Ocfs2-devel] [PATCH] ocfs2: fix a refcount condition checking

[Tao Ma]

Joel Becker wrote:
> On Thu, Feb 04, 2010 at 04:20:18PM -0500, Wengang Wang wrote:
>> index 06ccf6a..77ebb6e 100644
>> --- a/fs/ocfs2/file.c
>> +++ b/fs/ocfs2/file.c
>> @@ -1775,7 +1775,7 @@ static int ocfs2_prepare_inode_for_write(struct dentry *dentry,
>>  					 int *direct_io,
>>  					 int *has_refcount)
>>  {
>> -	int ret = 0, meta_level = 0;
>> +	int ret = 0, meta_level = 0, refcount = 0;
>>  	struct inode *inode = dentry->d_inode;
>>  	loff_t saved_pos, end;
>>  
>> @@ -1834,6 +1834,7 @@ static int ocfs2_prepare_inode_for_write(struct dentry *dentry,
>>  							       saved_pos,
>>  							       count,
>>  							       &meta_level);
>> +			refcount = 1;
>>  			if (has_refcount)
>>  				*has_refcount = 1;
>>  		}
> 
> 	Tao is right that this is redundant - we're setting two refcount
> bits, which is pointless.
> 	Here's the thing: we know that refcounted extents mean no direct
> io.  Instead of adding your 'refcount' bit or Tao's comment, why not
> just clear direct_io right here?
> 
> @@ -1834,6 +1834,8 @@ static int ocfs2_prepare_inode_for_write(struct dentry *dentry,
>  							       saved_pos,
>  							       count,
>  							       &meta_level);
>  			if (has_refcount)
>  				*has_refcount = 1;
> +			if (direct_io)
> +				*direct_io = 0;
>  		}
> 
> It's safe to clear if it exists - whether it was zero before, it must be
> zero now.
yeah, this is better.
btw, we also need to remove the latter lines.

-	if (has_refcount && *has_refcount == 1) {
-		*direct_io = 0;
-		break;
-}

Regards,
Tao