On Wed, Jul 24, 2024 at 11:40:07AM GMT, Jürgen Groß wrote:
> On 24.07.24 10:31, Lorenzo Stoakes wrote:
> > On Wed, Jul 24, 2024 at 10:14:12AM GMT, Jürgen Groß wrote:
> > > On 23.07.24 23:59, Lorenzo Stoakes wrote:
> > > > Arnd reported a significant build slowdown [0], which was bisected to the
> > > > series spanning commit 80fcac55385c ("minmax: relax check to allow
> > > > comparison between unsigned arguments and signed constants") to commit
> > > > 867046cc70277 ("minmax: relax check to allow comparison between unsigned
> > > > arguments and signed constants"), originating from the series "minmax:
> > > > Relax type checks in min() and max()." [1].
> >
> > [snip]
> >
> > > I can send a patch to simplify the problematic construct, but OTOH this
> > > will avoid only one particularly bad example.
> >
> > Thanks, appreciated but I am a little concerned that we might get stuck in
> > whack-a-mole here a bit. I'm pretty sure we've had previous patches that
> > have addressed invocation points, but obviously the underlying issue are
> > these macros which will keep cropping up again and again.
>
> The xen example seems to be one of the worst due to nesting of min3() and
> min(), so being de facto a min4().
>
> I think drivers/firmware/sysfb_simplefb.c has a similar problem, as it is
> nesting max() with max3(). Same applies to arch/x86/kernel/cpu/cacheinfo.c
> and multiple times to fs/xfs/libxfs/xfs_trans_resv.c.
>
> There are probably more such extreme cases.
>
Yeah to be clear, I am not opposed to these patches, I just don't want us to
lose sight of the need to fix the underlying problem if possible.
It feels like we are leaving the worst kind of landmine - a construct that you
simply wouldn't expect to cause massive build perf degradation - for others to
step on.
I suspect there are probably a few specific O(n^3) cases (as David pointed out)
that account for most of the problem and a bunch of other less problematic ones
that hit perhaps O(n^2) cases that add up.
>
> Juergen
