Re: ratelimit API: was: [RFC PATCH] printk: Introduce "store now but print later" prefix.

[Petr Mladek <pmladek@suse.com>]

On Thu 2019-03-07 03:24:25, Tetsuo Handa wrote:
> On 2019/03/06 19:04, Petr Mladek wrote:
> > I did not mean serializing. I meant to avoid printing the warnings
> > at all until OOM killer finishes its job.
> 
> But your ratelimit_reset() below requires serializing.
> 
> > 
> > 
> >> Also, both nopage_rs in warn_alloc() and oom_rs in oom_kill_process() are not
> >> working well. This is because ___ratelimit() function assumes that operations
> >> to be ratelimited complete fast enough to be able to repeat many times within
> >> a second. If one operation to be ratelimited takes many seconds (or even
> >> minutes), ___ratelimit() becomes always true and can not ratelimit at all.
> > 
> > The current ratelimiting is time driven. We might need an event
> > driven variant. It might even be done with the current
> > implementation if we add something like:
> > 
> > void ratelimit_reset(struct ratelimit_state *rs)
> > {
> > 	unsigned long flags;
> > 
> > 	raw_spin_irqsave(&rs->lock, flags);
> > 
> > 	rs->begin   = jiffies;
> > 	rs->printed = 0;
> > 
> > 	raw_spin_unlock_irqrestore(&rs->lock, flags);
> > }
> > 
> > We could call this when some event "solved" the problem.
> 
> This requires serialization among threads using "rs". I already
> proposed ratelimit_reset() for memcg's OOM problem at
> https://lkml.kernel.org/r/201810180246.w9I2koi3011358@www262.sakura.ne.jp
> but it was not accepted.

IMHO, the main problem was that the patch tried to work around
the ratelimit API weakness by a custom code.

I believe that using an improved/extended ratelimit API with
a sane semantic would be more acceptable.


> > It means that it makes sense to enable the related
> > ratelimited messages again because they would describe
> > another problem.
> 
> ___ratelimit() could also check number of not-yet-flushed
> printk() records (e.g. log_next_seq - console_seq <= $some_threshold).

The number is almost useless without more information, for example,
how fast the consoles are, how many lines will get filtered
by a console_loglevel, if the console_sem owner is sleeping,
how many messages are being added by other CPUs.

I believe that we do not really need it. The ratelimit_reset()
user should know when the messages can get skipped because
they describe the same situation again and again.

Best Regards,
Petr