From: Jiri Slaby <jslaby@suse.cz>
To: Joe Perches <joe@perches.com>
Cc: davem@davemloft.net, jirislaby@gmail.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ratelimit: check the condition in WARN_RATELIMIT first
Date: Fri, 17 Aug 2012 22:54:30 +0200 [thread overview]
Message-ID: <502EAF86.2040309@suse.cz> (raw)
In-Reply-To: <1345229139.10014.5.camel@joe2Laptop>
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
prev parent reply other threads:[~2012-08-17 20:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-17 13:42 [PATCH] ratelimit: check the condition in WARN_RATELIMIT first Jiri Slaby
2012-08-17 17:39 ` Joe Perches
2012-08-17 18:15 ` Jiri Slaby
2012-08-17 18:45 ` Joe Perches
2012-08-17 20:54 ` Jiri Slaby [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=502EAF86.2040309@suse.cz \
--to=jslaby@suse.cz \
--cc=davem@davemloft.net \
--cc=jirislaby@gmail.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).