Re: [PATCH 00/18] random: Include and resolve circular include dependency

Andy Shevchenko
2024-09-05T15:38:25Z

On Thu, Sep 05, 2024 at 03:03:24PM +0200, Uros Bizjak wrote:
> On Thu, Sep 5, 2024 at 2:41 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Thu, Sep 05, 2024 at 02:17:08PM +0200, Uros Bizjak wrote:
> > > There were several attempts to resolve circular include dependency
> > > after the addition of percpu.h: 1c9df907da83 ("random: fix circular
> > > include dependency on arm64 after addition of percpu.h"), c0842fbc1b18
> > > ("random32: move the pseudo-random 32-bit definitions to prandom.h") and
> > > finally d9f29deb7fe8 ("prandom: Remove unused include") that completely
> > > removes inclusion of <linux/percpu.h>.
> > >
> > > Due to legacy reasons, <linux/random.h> includes <linux/prandom.h>, but
> > > with the commit entry remark:
> > >
> > > --quote--
> > > A further cleanup step would be to remove this from <linux/random.h>
> > > entirely, and make people who use the prandom infrastructure include
> > > just the new header file. That's a bit of a churn patch, but grepping
> > > for "prandom_" and "next_pseudo_random32" "struct rnd_state" should
> > > catch most users.
> > >
> > > But it turns out that that nice cleanup step is fairly painful, because
> > > a _lot_ of code currently seems to depend on the implicit include of
> > > <linux/random.h>, which can currently come in a lot of ways, including
> > > such fairly core headfers as <linux/net.h>.
> > >
> > > So the "nice cleanup" part may or may never happen.
> > > --/quote--
> > >
> > > __percpu tag is currently defined in include/linux/compiler_types.h,
> > > so there is no direct need for the inclusion of <linux/percpu.h>.
> > > However, in [1] we would like to repurpose __percpu tag as a named
> > > address space qualifier, where __percpu macro uses defines from
> > > <linux/percpu.h>.
> > >
> > > This patch series is the "nice cleanup" part, and allows us to finally
> > > include <linux/percpu.h> in prandom.h.
> > >
> > > The whole series was tested by compiling the kernel for x86_64 allconfig
> > > and some popular architectures, namely arm64 defconfig, powerpc defconfig
> > > and loongarch defconfig.
> >
> > Obvious question(s) is(are):
> > 1) have you seen the Ingo's gigantic patch series towards resolving issues with
> > the headers?
> > 2) if not, please look at the preliminary work and take something from there, I
> > believe there are many useful changes already waiting for a couple of years to
> > be applied.
> >
> > Because I haven't found any references nor mentions of that in the cover letter
> > here and explanation why it was not taking into consideration.
>
> I am aware of the series, but the patch takes only a small bite of it
> and specifically resolves the inclusion of <linux/prandom.h> from
> linux/random.h. The series fixes the missing inclusion of
> <linux/prandom.h> in files that use pseudo-random function and it was
> not meant to be a general header cleanup. The end goal is to allow us
> to include <linux/percpu.h> in linux/prandom.h - which uses __percpu
> tag without the correct include.

Thank you for elaboration, it's all clear now.

> Thus, the patch series is only tangentially connected to Ingo's patch series.

--
With Best Regards,
Andy Shevchenko