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;
Message-ID: <541d89b3aa2092382405672b70b58b5b3d263f1e.camel@svanheule.net>
Subject: Re: [PATCH v3 1/4] cpumask: Fix invalid uniprocessor mask assumption
From:   Sander Vanheule <sander@svanheule.net>
To:     Yury Norov <yury.norov@gmail.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Valentin Schneider <vschneid@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Marco Elver <elver@google.com>, linux-kernel@vger.kernel.org,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Tue, 07 Jun 2022 14:06:38 +0200
In-Reply-To: <Yp5SJAkq8whC8EBI@yury-laptop>
References: <cover.1654410109.git.sander@svanheule.net>
         <cc1f7d3334348cccbf9fde091015a802f379a9be.1654410109.git.sander@svanheule.net>
         <Yp5SJAkq8whC8EBI@yury-laptop>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.2 (3.44.2-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2022-06-06 at 12:14 -0700, Yury Norov wrote:
> On Sun, Jun 05, 2022 at 08:22:38AM +0200, Sander Vanheule wrote:
> > On uniprocessor builds, any CPU mask is assumed to contain exactly one
> > CPU (cpu0). This assumption ignores the existence of empty masks,
> > resulting in incorrect behaviour.
> > cpumask_first_zero(), cpumask_next_zero(), and for_each_cpu_not() don't
> > provide behaviour matching the assumption that a UP mask is always "1",
> > and instead provide behaviour matching the empty mask.
> >=20
> > Drop the incorrectly optimised code and use the generic implementations
> > in all cases.
> >=20
> > Signed-off-by: Sander Vanheule <sander@svanheule.net>
> > ---

[...]

> > -static inline unsigned int cpumask_local_spread(unsigned int i, int no=
de)
> > -{
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return 0;
> > -}
> > -
> > -static inline int cpumask_any_and_distribute(const struct cpumask *src=
1p,
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 const struct cpumask *src2p) {
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return cpumask_first_and(src=
1p, src2p);
> > -}
> > -
> > -static inline int cpumask_any_distribute(const struct cpumask *srcp)
> > -{
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return cpumask_first(srcp);
> > -}
>=20
> It looks like cpumask_local_spread, cpumask_any_and_distribute and
> cpumask_any_distribute were correct and better optimized in UP case.
> cpumask_local_spread - for sure. I think it's worth keeping them
> optimized.

Yes, these were correct and we can keep them. I will have to add an #ifded =
CONFIG_SMP (or #if
NR_CPUS > 1) guard in lib/cpumask.c around these functions, so they don't c=
ollide with the inlined
UP versions.

Best,
Sander