lkml.org 
[lkml]   [2012]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] ratelimit: check the condition in WARN_RATELIMIT first
On 08/17/2012 08:45 PM, Joe Perches wrote:
> On Fri, 2012-08-17 at 20:15 +0200, Jiri Slaby wrote:
>> On 08/17/2012 07:39 PM, Joe Perches wrote:
>>> On Fri, 2012-08-17 at 15:42 +0200, Jiri Slaby wrote:
>>>> Before calling __ratelimit in __WARN_RATELIMIT, check the condition
>>>> first. When this check was not there, we got constant income of:
>>>> tty_init_dev: 60 callbacks suppressed
>>>> tty_init_dev: 59 callbacks suppressed
>>> []
>>>> diff --git a/include/linux/ratelimit.h b/include/linux/ratelimit.h
>>> []
>>>> @@ -49,8 +49,9 @@ extern int ___ratelimit(struct ratelimit_state *rs, const char *func);
>>>> #define __WARN_RATELIMIT(condition, state, format...) \
>>>> ({ \
>>>> int rtn = 0; \
>>>> - if (unlikely(__ratelimit(state))) \
>>>> - rtn = WARN(condition, format); \
>>>> + int __rtcond = !!condition; \
>>>> + if (unlikely(__rtcond && __ratelimit(state))) \
>>>> + rtn = WARN(__rtcond, format); \
>>>> rtn; \
>>>> })
>>>>
>>>
>>> Hi Jiri.
>>>
>>> This seems fine to me but are there any conditions that
>>> are computationally expensive?
>>
>> It's not about expensiveness of the computation. The complexity remained
>> the same except I moved the computation one layer up.
>
> If ratelimit(state) is not true, condition wasn't tested
> or performed at all. With this change, it's always done.

Ah, you meant this. Actually this was wrong/unexpected. When devs pass
something to a function/macro they expect it to be evaluated. Exactly once.

Like in this (maybe not so good) code:
void put_ref(int refcnt) {
WARN_RATELIMIT(!--refcnt, "refcnt reached 0 unexpectedly");
}

You want the refcnt to be decremented no matter what ratelimit() returns.

thanks,
--
js
suse labs


\
 
 \ /
  Last update: 2012-08-17 23:21    [W:0.033 / U:0.836 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site