Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 20 Jun 2022 11:07:20 -0400
From:   Kent Overstreet <kent.overstreet@gmail.com>
To:     David Laight <David.Laight@ACULAB.COM>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "pmladek@suse.com" <pmladek@suse.com>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        "enozhatsky@chromium.org" <enozhatsky@chromium.org>,
        "linux@rasmusvillemoes.dk" <linux@rasmusvillemoes.dk>,
        "willy@infradead.org" <willy@infradead.org>
Subject: Re: [PATCH v4 00/34] Printbufs - new data structure for building
 strings
Message-ID: <20220620150514.3tjy5dv7pv5frcwd@moria.home.lan>
References: <20220620004233.3805-1-kent.overstreet@gmail.com>
 <0a5901f8460f452a89c9b0cda32fb833@AcuMS.aculab.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0a5901f8460f452a89c9b0cda32fb833@AcuMS.aculab.com>
Precedence: bulk

On Mon, Jun 20, 2022 at 04:19:31AM +0000, David Laight wrote:
> From: Kent Overstreet
> >   Rasmus pointed out that -fno-strict-aliasing is going to cause gcc to generate
> >   nasty code, and indeed it unfortunately does but according to worst case
> >   scenario microbenchmarks it's not a problem for actual performance.
> 
> Just copy some of the structure members to local variables
> and, if necessary, write them back at the end.

You must not have read any of the code - half the point of this patch series is
implementing proper helpers for printing chars, strings of bytes, etc. and that
doesn't work if we're not using actual types.

> >      printk("%pd", dentry);
> > 
> >    becomes
> > 
> >      printk("%pf(%p)", prt_dentry, dentry);
> > 
> >    My OOM debugging & reporting patch series that builds off of this uses this
> >    to solve a very real problem that Michal Hocko brought up at LSF - with this
> >    we write shrinkers_to_text(), slab_to_text() which can _also_ now be used for
> >    reporting in debugfs (which Roman has been working on), as well as in the
> >    show_mem() report - the "%pf()" syntax lets us print the output of those
> >    functions without allocating (and having to preallocate) a separate buffer.
> 
> I really think that is a bad idea.
> printk() already uses a lot of stack, anything doing a recursive
> call is just making that worse.
> Especially since these calls can often be in error paths
> which are not often tested and can already be on deep stacks.

We went over this before - this patch series drastically reduces stack usage of
sprintf by eliminating a bunch of stack allocated buffers. Do try to keep up...