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 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.

> ratelimit(state) isn't
> and this will now always do condition.
>
> (looks instead of speculates)
>
> There's 1 current use of WARN_RATELIMIT and there's
> a condition of 1 so there's no problem here.

There is going to be one more in monday's -next. I've just added one to
the TTY code. The thing is that when you call ratelimit(state) it will
emit how many times you have called that function like I described in
the changelog:
tty_init_dev: 60 callbacks suppressed

Even when the condition is always false. Hence I added the condition to
the if and lazy evaluation will take care and ratelimit() won't be
called at all...

> __WARN_RATELIMIT is pretty stupid.
> It's only called from WARN_RATELIMIT.
> I think it shouldn't exist at all.
>
> Maybe something like this?

Yup, something like that looks OK to me.

thanks,
--
js
suse labs


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