[PATCH] xfs: prevent deadlock in xfs_qm_shake()

Andi Kleen andi at firstfloor.org
Sat May 30 05:43:54 CDT 2009


Christoph Hellwig <hch at infradead.org> writes:
>> 
>> diff --git a/fs/xfs/linux-2.6/kmem.h b/fs/xfs/linux-2.6/kmem.h
>> index af6843c..d8d2321 100644
>> --- a/fs/xfs/linux-2.6/kmem.h
>> +++ b/fs/xfs/linux-2.6/kmem.h
>> @@ -103,7 +103,7 @@ extern void *kmem_zone_zalloc(kmem_zone_t *, unsigned int __nocast);
>>  static inline int
>>  kmem_shake_allow(gfp_t gfp_mask)
>>  {
>> -	return (gfp_mask & __GFP_WAIT) != 0;
>> +	return ((gfp_mask & __GFP_WAIT && gfp_mask & __GFP_FS) != 0;
>
> Looks good to me.  But this could be written simpler as:
>
> 	return ((gfp_mask & (__GFP_WAIT|__GFP_FS)) != 0;

That's actually not equivalent. You would need to write

(gfp_mask & (__GFP_WAIT|__GFP_FS)) == (__GFP_WAIT|__GFP_FS)

which is not simpler. 

BTW you can usually trust gcc to do all the obvious logical
simplifications, so it's best to just write the most readable code
(like Felix's original version) And also it can depend on the target
what is actually better in machine language, and gcc has more
knowledge about that than the programmer.

int f(int x)
{
        return x & 1 && x & 2;  
}

gives for gcc 4.3

 andl    $3, %edi
 xorl    %eax, %eax
 cmpl    $3, %edi
 sete    %al
 ret

-Andi


-- 
ak at linux.intel.com -- Speaking for myself only.




More information about the xfs mailing list