Re: [RFC PATCH v1 00/25] printk: new implementation

[Eugeniu Rosca]

Hi John,

On Wed, Jan 22, 2020 at 11:33:12AM +0100, John Ogness wrote:
> On 2020-01-22, Eugeniu Rosca <erosca@xxxxxxxxxxxxxx> wrote:
> > So, what's specific to R-Car3, based on my testing, is that the issue
> > can only be reproduced if the printk storm originates on CPU0 (it does
> > not matter if from interrupt or task context, both have been
> > tested). If the printk storm is initiated on any other CPU (there are
> > 7 secondary ones on R-Car H3), there is no regression in the audio
> > quality/latency.
> >
> > I cannot fully explain this empirical observation, but it directs my
> > mind to the following workaround, for which I have a PoC:
> >  - employ vprintk_safe() any time CPU0 is the owner/caller of printk
> >  - tie CPU0-private printk internal IRQ workers to another CPU
> >
> > The above makes sure nothing is printed to the serial console on
> > behalf of CPU0. I don't even hope this to be accepted by community,
> > but can you please share your opinion the idea itself is sane?
> 
> It is a problem-specific hack. You will need to be certain that CPU1-7
> will never have problems with console printing storms.
> 
> Be aware that vprintk_safe() is not particularly reliable in many crash
> scenarios. If seeing oops output on the console is important, this can
> be a risky hack.
> 
> Also, be aware that it has its own config option for the safe buffer
> size: PRINTK_SAFE_LOG_BUF_SHIFT

The warnings and pitfalls are much appreciated. Also, this whole
discussion has been referenced in the recently started communication
thread with Renesas, to raise the awareness of what looks to be not
only the limitation of Renesas BSP, but the mainline kernel in the
first place.

> >> The printk rework focusses on making printk non-interfering by
> >> decoupling console printing from printk() callers. However, the
> >> console printing itself will still do just as much interrupt
> >> disabling as before. That is driver-related, not printk-related.
> >
> > I didn't dive into the internals of this series, but decoupling the
> > execution context of the serial driver from the execution context of
> > the printk callers sounds very good to me (this is what i try to
> > achieve via vanilla vprintk_safe). I wonder if it's easier to remove
> > CPU0 from equation with this series applied.
> 
> Yes, it would be quite easy. The console printers run as dedicated
> kthreads. It is only a matter of setting the CPU affinity for the
> related kthread.

Confirmed. Below two lines do the job (v5.4.13-rt7+).

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0605a74ad76b..7bc2cdabf516 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2733,11 +2733,12 @@ static int __init init_printk_kthread(void)
 {
        struct task_struct *thread;

-       thread = kthread_run(printk_kthread_func, NULL, "printk");
+       thread = kthread_create_on_cpu(printk_kthread_func, NULL, 7, "printk");
        if (IS_ERR(thread)) {
                pr_err("printk: unable to create printing thread\n");
                return PTR_ERR(thread);
        }
+       wake_up_process(thread);

        return 0;
 }

>
> >> The linux-rt patches (which include this printk rework) *are* being
> >> ported to mainline now. My recommendation is to continue using the
> >> linux-rt patches (with PREEMPT_RT=y) until PREEMPT_RT is available
> >> mainline.

This has been relayed to Renesas. Thanks.

> >
> > If there is any roadmap publicly available, I would appreciate a
> > reference.
> 
> I am only aware of the quilt "series" file [0] that is roughly
> documenting the status of the effort.
> 
> John Ogness
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git/tree/patches/series?h=linux-5.4.y-rt-patches

Great.

-- 
Best Regards
Eugeniu Rosca