On Wed, May 16, 2012 at 12:49 AM, walter harms <wharms@xxxxxx> wrote:
>
>
> Am 15.05.2012 16:18, schrieb Boaz Harrosh:
>> On 05/15/2012 04:57 PM, Dan Carpenter wrote:
>>
>>> On Tue, May 15, 2012 at 04:48:23PM +0300, Boaz Harrosh wrote:
>>>> On 05/14/2012 10:45 PM, Dan Carpenter wrote:
>>>>
>>>>> Obviously we should check for NULL here instead of IS_ERR().
>>>>>
>>>>> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>>>>>
>>>>> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
>>>>> index ba3019f..233beea 100644
>>>>> --- a/fs/nfs/idmap.c
>>>>> +++ b/fs/nfs/idmap.c
>>>>> @@ -644,14 +644,14 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
>>>>>
>>>>> /* msg and im are freed in idmap_pipe_destroy_msg */
>>>>> msg = kmalloc(sizeof(*msg), GFP_KERNEL);
>>>>> - if (IS_ERR(msg)) {
>>>>> - ret = PTR_ERR(msg);
>>>>> + if (!msg) {
>>>>
>>>>
>>>> While at it please put an unlikely()
>>>>
>>>
>>> Normally we wouldn't put an unlikely() here. It makes the code
>>> less readable and it's not going to affect benchmarks. But I can
>>> add one if people prefer.
>>>
>>
>>
>> Personally It makes it more readable for me. It's like a statement:
>> "error, always slow-path case here". I have brain parsers set
>> for these.
>>
>> Specifically here the if () is very small but if it is more code
>> it is exactly where it counts. But as a general rule I like the
>> error/slow-path case to be in an unlikely(). Later someone
>> might add more code which will matter.
>>
My impression is these hints are useful iif the code is on performance
critical path like scheduler, path walking, etc. Apparently nfs idmap
isn't the case.
Thanks,
Tao
>>> regards,
>>> dan carpenter
>>>
>>
>>
>
> Not long a go we had a hint from one of the developers that this likely()-stuff
> should be used in the scheduler and has no use outside.
>
> re,
> wh
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
