Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 25 Sep 2020 10:13:20 +0200
From:   Petr Mladek <pmladek@suse.com>
To:     Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc:     John Ogness <john.ogness@linutronix.de>,
        Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH printk 3/5] printk: use buffer pool for sprint buffers
Message-ID: <20200925081319.GJ29288@alley>
References: <20200922153816.5883-1-john.ogness@linutronix.de>
 <20200922153816.5883-4-john.ogness@linutronix.de>
 <20200924061746.GF577@jagdpanzerIV.localdomain>
 <20200924085445.GK6442@alley>
 <20200924090653.GC541@jagdpanzerIV.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200924090653.GC541@jagdpanzerIV.localdomain>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Thu 2020-09-24 18:06:53, Sergey Senozhatsky wrote:
> On (20/09/24 10:54), Petr Mladek wrote:
> > > Just a question:
> > > 
> > > Can dynamic_textbuf be a PER_CPU array of five textbuf[1024] buffers
> > > (for normal printk, nmi, hard irq, soft irq and one extra buffer for
> > > recursive printk calls)?
> > 
> > That would be my preferred fallback when the approach with
> > vsprintf(NULL, ) is not acceptable for some reasons.
> 
> OK.
> 
> > But I still think that calling vsprintf(NULL, ) is the most trivial
> > and good enough solution.
> 
> It's probably good enough.
> 
> > IMHO, the solution with per-CPU buffers is not that trivial, for
> > example:
> > 
> >   What if recursive printk() is interrupted by NMI and it causes
> >   yet another recursion?
> > 
> >   Is one level of recursion enough?
> 
> We might try the current approach - when we, for example, have
> recursion in printk_safe() we just end up writing data to the
> same per-CPU buffer. We need to limit the depth of recursion
> one way or another. With per-CPU counter we will just bail out
> of "deeply recursive printk" without attempting to store its
> messages; with the buffers approach we will write the data to
> a static buffer and see how badly it will be overlapped at the
> end. Just a thought.

It makes some sense but it has just another bunch of problems:

   + require some tricky code [*]
   + temporary buffers need extra care in panic()
   + does not guarantee that there is enough stack for the recursion

I think that limiting recursion to some level (3 or 5?) is much
easier and provide similar protection.


[*] We could reuse printk_safe but we would need to maintain it.
    In fact, the code might be easier because it will not be needed
    to read the buffer from other CPU. But it would need to be
    tricky anyway.

Best Regards,
Petr