On (20/12/04 17:10), Petr Mladek wrote:
[..]
> char *get_printk_counter_by_ctx()
> {
> int ctx = 0;
>
> if (in_nmi)
> ctx = 1;
>
> if (!printk_percpu_data_ready())
> return &printk_count_early[ctx];
>
> return this_cpu_ptr(printk_count[ctx]);
> }
>
> > +
> > + return count;
> > +}
> > +
> > +static bool printk_enter(unsigned long *flags)
> > +{
> > + char *count;
> > +
> > + local_irq_save(*flags);
> > + count = get_printk_count();
> > + /* Only 1 level of recursion allowed. */
>
> We should allow at least some level of recursion. Otherwise, we would
> not see warnings printed from vsprintf code.
One level of recursion seems reasonable on one hand, but on the other
hand I also wonder if we can get useful info from recursion levels 2
and higher. Would be great to maybe list possible scenarios. vsprintf()
still call re-enter printk() -- WARN_ONCE()-s in the code -- external
format specifiers handlers also can. Would we need to let two levels of
recursion printk->vsprintf->printk->???->printk or one is just enough?
It also would make sense to add the lost messages counter to per-CPU
recursion counter struct, to count the number of times we bailout
of printk due to recursion limit. So that we'll at least have
"%d recursive printk messages lost" hints.
Overall...
I wonder where does the "limit printk recursion" come from? printk_safe
doesn't impose any strict limits (print_context is limited, but still)
and we've been running it for years now; have we ever seen any reports
of printk recursion overflows?
> > + if (*count > 1) {
> > + local_irq_restore(*flags);
> > + return false;
> > + }
> > + (*count)++;
> > +
> > + return true;
> > +}
>
> This should be unified with printk_context, printk_nmi_enter(),
> printk_nmi_exit(). It does not make sense to have two separate
> printk context counters.
Agreed.
> Or is there any plan to remove printk_safe and printk_context?
That's a good point. This patch set and printk_safe answer the
same question in different ways, as far as I understand it. The
question is "Why do we want to track printk recursion"? This patch
set merely wants to, correct me if I'm wrong, avoid the very deep
vprintk_store() recursion stacks (which is a subset of printk()
recursion superset):
vprintk_store()
{
if (!printk_enter())
return;
vsprintf/prb
print_exit();
}
And that's pretty much it, at least for the time being.
printk_safe()'s answer is - we don't want to re-enter parts of
the kernel that sit in the core, behind the scenes, and that are
not ready to be re-entered. Things like
printk()
down_console_sem()
down()
raw_spin_lock_irqsave(&sem->lock)
printk()
down_console_sem()
down()
raw_spin_lock_irqsave(&sem->lock)
-ss
