From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750909AbcGOEAt (ORCPT ); Fri, 15 Jul 2016 00:00:49 -0400 Received: from mail.skyhub.de ([78.46.96.112]:55073 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750705AbcGOEAq (ORCPT ); Fri, 15 Jul 2016 00:00:46 -0400 Date: Fri, 15 Jul 2016 06:00:49 +0200 From: Borislav Petkov To: Andrew Morton Cc: LKML , Franck Bui , Greg Kroah-Hartman , Ingo Molnar , Linus Torvalds , Peter Zijlstra , Steven Rostedt , Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= Subject: Re: [PATCH -v4 1/2] ratelimit: Extend to print suppressed messages on release Message-ID: <20160715040049.GA13899@nazgul.tnic> References: <1467969530-5215-1-git-send-email-bp@alien8.de> <1467969530-5215-2-git-send-email-bp@alien8.de> <20160714132936.e04a3ca668b67f3afb0a88c0@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160714132936.e04a3ca668b67f3afb0a88c0@linux-foundation.org> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andrew, thanks for taking a look. On Thu, Jul 14, 2016 at 01:29:36PM -0700, Andrew Morton wrote: > Why? What's driving this? What are the benefits to our users? Are > there any downsides or back-compatibility issues? > > I see from the code that this is not actually enabled by default. The > client code must use ratelimit_set_flags() to select this behaviour, > and the second patch uses this. Please include all such info in the > changelog. How about: "This use case is aimed at short-termed, burst-like users of the ratelimiting facility for which we want to output the suppressed lines stats only once, after it has been disposed of. For an example, see usage in /dev/kmsg." ? > > Separated from a previous patch by Linus. > > > > Also, make the ON_RELEASE image not use "callbacks" as it is misleading. > > "image"? Bah, it should say "Also, change the printk line we issue on release to not use "callbacks" as it is misleading. We're not suppressing callbacks but printk calls." > > @@ -46,12 +46,14 @@ int ___ratelimit(struct ratelimit_state *rs, const char *func) > > rs->begin = jiffies; > > > > if (time_is_before_jiffies(rs->begin + rs->interval)) { > > - if (rs->missed) > > - printk(KERN_WARNING "%s: %d callbacks suppressed\n", > > - func, rs->missed); > > + if (rs->missed) { > > + if (!(rs->flags & RATELIMIT_MSG_ON_RELEASE)) { > > + pr_warn("%s: %d callbacks suppressed\n", func, rs->missed); > > + rs->missed = 0; > > + } > > + } > > hm, what's the difference between an output line being suppressed and a > callback being suppressed? I think I've forgotten how this code works ;) Right, ___ratelimit() gets as @func arg the name of the current calling function: #define __ratelimit(state) ___ratelimit(state, __func__) I'm strongly assuming this is the "callback" ___ratelimit() is talking about :-) In our case, we don't have callbacks but /dev/kmsg users and I thought the most generic way of referring to them would be by not doing so at all but simply talking about output lines being suppressed. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --