xfs
[Top] [All Lists]

Re: [PATCH] xfs: prevent deadlock in xfs_qm_shake()

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: prevent deadlock in xfs_qm_shake()
From: Andi Kleen <andi@xxxxxxxxxxxxxx>
Date: Sat, 30 May 2009 12:43:54 +0200
Cc: Felix Blyakher <felixb@xxxxxxx>, Hedi Berriche <hedi@xxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20090529192529.GA1599@xxxxxxxxxxxxx> (Christoph Hellwig's message of "Fri, 29 May 2009 15:25:29 -0400")
References: <1243620631-10749-1-git-send-email-felixb@xxxxxxx> <1243620631-10749-2-git-send-email-felixb@xxxxxxx> <20090529192529.GA1599@xxxxxxxxxxxxx>
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/22.3 (gnu/linux)
Christoph Hellwig <hch@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx -- Speaking for myself only.

<Prev in Thread] Current Thread [Next in Thread>