From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dmitry Safonov <dima@arista.com>
Subject: Re: [PATCHv3] lib/ratelimit: Lockless ratelimiting
Date: Thu, 02 Aug 2018 16:19:15 +0100
Message-ID: <1533223155.2679.136.camel@arista.com>
References: <20180703225628.25684-1-dima@arista.com>
         <1531789154.18720.3.camel@arista.com>
         <CAHp75VdmQ1YLzWpYb7878zKOCCyOEicdLAtpohbmY2hL1h1RNg@mail.gmail.com>
         <20180720150948.nv57fjmmrmo2r7rx@pathway.suse.cz>
         <1532100816.18720.44.camel@arista.com>
         <20180801214855.3ae1abb9@vmware.local.home>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <20180801214855.3ae1abb9@vmware.local.home>
Sender: linux-kernel-owner@vger.kernel.org
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Petr Mladek <pmladek@suse.com>, Andy Shevchenko <andy.shevchenko@gmail.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Arnd Bergmann <arnd@arndb.de>, David Airlie <airlied@linux.ie>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jani Nikula <jani.nikula@linux.intel.com>, Joonas Lahtinen <joonas.lahtinen@linux.intel.com>, Rodrigo Vivi <rodrigo.vivi@intel.com>, Theodore Ts'o <tytso@mit.edu>, Thomas Gleixner <tglx@linutronix.de>, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
List-Id: dri-devel@lists.freedesktop.org

Hi Steven,
Thanks for your reply,

On Wed, 2018-08-01 at 21:48 -0400, Steven Rostedt wrote:
> I'm just catching up from my vacation. What about making rs->missed
> into an atomic, and have:
> 
> 	if (!raw_spin_trylock_irqsave(&rs->lock, flags)) {
> 		atomic_inc(&rs->missed);
> 		return 0;
> 	}
> 
> ?

Uhm. Do you mean as a preparation patch to split this on two patches?
Because it will not solve the issue where one CPU has taken rs->lock,
and is updating rs->printed, checking burst and whatnot; while the
second CPU will loose the message which was even *under* burst limit.

I.e.: there are enough of printk_ratelimit() users in tree and a
message from one can be suppressed, while shouldn't.

> You would also need to do:
> 
> 	if (time_is_before_jiffies(rs->begin + rs->interval)) {
> 		int missed = atomic_xchg(&rs->missed, 0);
> 		if (missed) {
> 
> So that you don't have a race between checking rs->missed and setting
> it
> to zero.

-- 
Thanks,
             Dmitry