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: <e5e85517a918b67ca8315a88ae9a8c1c6ce4da0f.camel@svanheule.net>
Subject: Re: [RFC] issue with cpumask for UniProcessor
From:   Sander Vanheule <sander@svanheule.net>
To:     Saurabh Sengar <ssengar@linux.microsoft.com>,
        ssengar@microsoft.com, mikelley@microsoft.com,
        linux-kernel@vger.kernel.org
Date:   Mon, 08 Aug 2022 19:34:53 +0200
In-Reply-To: <1659975821-30762-1-git-send-email-ssengar@linux.microsoft.com>
References: <e78c55ecb98172356248a7a89da501479ead6ae0.1659077534.git.sander@svanheule.net>
         <1659975821-30762-1-git-send-email-ssengar@linux.microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.3 (3.44.3-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

Hi Saurabh,

On Mon, 2022-08-08 at 09:23 -0700, Saurabh Sengar wrote:
>=20
> Hi,
>=20
> I am working on a UniProcessor system with latest linux-next kernel
> (20220803).
> I observed two files "shared_cpu_map=E2=80=9D and =E2=80=9Cshared_cpu_lis=
t=E2=80=9D are missing
> for L3 cache (/sys/devices/system/cpu/cpu0/cache/index3). This causes lsc=
pu
> version 2.34 to segfault. On further digging I figured below is the commi=
t
> which introduced this problem.
>=20
> https://lore.kernel.org/lkml/e78c55ecb98172356248a7a89da501479ead6ae0.165=
9077534.git.sander@svanheule.net/
>=20

This is the v5 of the patch, which sadly isn't the version that got merged.=
 The
commit that's triggering your issue is b81dce77cedc ("cpumask: Fix invalid
uniprocessor mask assumption"), which is patch v4.

https://lore.kernel.org/lkml/86bf3f005abba2d92120ddd0809235cab4f759a6.16567=
77646.git.sander@svanheule.net/

>=20
> I am not 100% certain what the proper fix for it is, but below changes fi=
x
> this issue. I understand above patch is already confirmed for linux kerne=
l
> 6.0, please suggest if we need fixing this in 6.0.
>=20
> Regards,
> Saurabh
>=20
>=20
>=20
> diff --git a/lib/cpumask.c b/lib/cpumask.c
> index b9728513a4d4..81fc2e35b5b1 100644
> --- a/lib/cpumask.c
> +++ b/lib/cpumask.c
> @@ -16,10 +16,14 @@
> =C2=A0 */
> =C2=A0unsigned int cpumask_next(int n, const struct cpumask *srcp)
> =C2=A0{
> +#if NR_CPUS =3D=3D 1
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return n+1;
> +#else

This is ignoring the provided cpumask again, which was exactly what my patc=
h
fixed. If the mask is empty, then cpumask_next(-1, mask) should return (at
least) 1, not 0.

I think the problem could be caused by cpumask_next() getting an empty mask=
.
Then the real issue is would be that a certain mask is empty when it should=
n't
be, which was compensated by the old code's built-in assumption that a cpum=
ask
couldn't be empty.

My MIPS testing system doesn't have these L3 maps, and "shared_cpu_map" and
"shared_cpu_list" are present for index0 and index1. I would propose that y=
ou
look for the point where the files should be created, and check how
cpumask_next() is involved, to find the actual cause of this problem.

Best,
Sander

> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* -1 is a legal arg here. */
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (n !=3D -1)
> =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 cpumask_check(n);
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return find_next_bit(cpumask_b=
its(srcp), nr_cpumask_bits, n + 1);
> +#endif
> =C2=A0}
> =C2=A0EXPORT_SYMBOL(cpumask_next);