Re: [PATCH v5 0/5] cpumask: fix invalid uniprocessor assumptions
From: Yury Norov
Date: Sun Jul 31 2022 - 11:28:29 EST
On Sun, Jul 31, 2022 at 03:02:55PM +0200, Sander Vanheule wrote:
> On Sat, 2022-07-30 at 11:15 -0700, Yury Norov wrote:
> > On Fri, Jul 29, 2022 at 09:01:17AM +0200, Sander Vanheule wrote:
> > > On uniprocessor builds, it is currently assumed that any cpumask will
> > > contain the single CPU: cpu0. This assumption is used to provide
> > > optimised implementations.
> > >
> > > The current assumption also appears to be wrong, by ignoring the fact
> > > that users can provide empty cpumasks. This can result in bugs as
> > > explained in [1] - for_each_cpu() will run one iteration of the loop
> > > even when passed an empty cpumask.
> > >
> > > This series introduces some basic tests, and updates the optimisations
> > > for uniprocessor builds.
> > >
> > > The x86 patch was written after the kernel test robot [2] ran into a
> > > failed build. I have tried to list the files potentially affected by the
> > > changes to cpumask.h, in an attempt to find any other cases that fail on
> > > !SMP. I've gone through some of the files manually, and ran a few cross
> > > builds, but nothing else popped up. I (build) checked about half of the
> > > potientally affected files, but I do not have the resources to do them
> > > all. I hope we can fix other issues if/when they pop up later.
> > >
> > > [1] https://lore.kernel.org/all/20220530082552.46113-1-sander@xxxxxxxxxxxxx/
> > > [2] https://lore.kernel.org/all/202206060858.wA0FOzRy-lkp@xxxxxxxxx/
> >
> > Hi Sander,
> >
> > I tried to apply it on top of bitmap-for next, and there are many conflicts
> > with already pulled patches. There's nothing really scary, just functions
> > changed their prototypes and locations. Can you try your series on top of
> > bitmap-for-next from git@xxxxxxxxxx:/norov/linux.git (or just -next)?
> >
> > I'm asking you to do it instead of doing myself because I don't want to
> > screwup your code accidentally and because many cpumask functions in -next
> > are moved to the header, and it would be probably possible to avoid building
> > cpumask.o in UP case.
> >
> > Briefly looking into the -next code, cpumask.c hosts only cpumask_next_wrap()
> > that is not overwritten by UP code, and in UP case it can be simplified.
>
> Sure. I've rebased my patches and added a UP-version for cpumask_next_wrap(), so
> cpumask.o doesn't have to be built anymore in that case.
Thanks!
> How would you like to proceed with these patches? It's fine by me if you take
> them through your tree to avoid more merge conflicts with your changes, but then
> Andrew woud have to drop the series from mm-nonmm-stable.
I also thing that it should go through bitmap thee. Andrew, are you OK with
that?