linux-kernel - Re: [PATCH v12 8/9] cpumask: Add initialiser CPUMASK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aM1vcBuOSh8OV7mN@yury>
Date: Fri, 19 Sep 2025 10:58:02 -0400
From: Yury Norov <yury.norov@...il.com>
To: Gabriele Monaco <gmonaco@...hat.com>
Cc: linux-kernel@...r.kernel.org, Frederic Weisbecker <frederic@...nel.org>
Subject: Re: [PATCH v12 8/9] cpumask: Add initialiser CPUMASK_NULL to use
 cleanup helpers

On Wed, Sep 17, 2025 at 08:34:54AM -0400, Yury Norov wrote:
> On Wed, Sep 17, 2025 at 02:08:06PM +0200, Gabriele Monaco wrote:
> > On Wed, 2025-09-17 at 07:38 -0400, Yury Norov wrote:
> > > On Wed, Sep 17, 2025 at 09:51:47AM +0200, Gabriele Monaco wrote:
> > > > According to what I can understand from the standard, the C list
> > > > initialisation sets to the default value (e.g. 0) all elements not
> > > > present in the initialiser. Since in {} no element is present, {}
> > > > is not a no-op but it initialises the entire cpumask to 0.
> > > > 
> > > > Am I missing your original intent here?
> > > > It doesn't look like a big price to pay, but I'd still reword the
> > > > sentence to something like:
> > > > "and a valid struct initializer when CPUMASK_OFFSTACK is disabled."
> > > 
> > > The full quote is:
> > > 
> > >   So define a CPUMASK_NULL macro, which allows to init struct cpumask
> > >   pointer with NULL when CPUMASK_OFFSTACK is enabled, and effectively
> > >   a no-op when CPUMASK_OFFSTACK is disabled.
> > > 
> > > If you read the 'which allows' part, it makes more sense, isn't?
> > 
> > Alright, my bad for trimming the sentence, what I wanted to highlight
> > is that with !CPUMASK_OFFSTACK this CPUMASK_NULL becomes something like
> > 
> >   struct cpumask mask[1] = {};
> > 
> > which, to me, doesn't look like a no-op as the description suggests,
> > but an initialisation of the entire array.
> > 
> > Now I'm not sure if the compiler would be smart enough to optimise this
> > assignment out, but it doesn't look obvious to me.
> > 
> > Unless you were meaning the __free() becomes a no-op (which is true but
> > out of scope in this version of the patch), I would avoid mentioning
> > the no-op altogether.
> > 
> > Am I missing something and that initialisation is proven to be compiled
> > out?
> 
> When you create a non-initialized variable on stack, compiler does
> nothing about it, except for adjusting an argument to brk() emitted in
> the function prologue.
> 
> In this case, non-initialized struct cpumask is already on stack, and
> switching from
> 
>         struct cpumask mask[1];
> 
> to
> 
>         struct cpumask mask[1] = {};
> 
> is really a no-op.

Alright... The above is correct for optimization levels > 0.
With -O0, 2nd version really makes GCC to initialize the array.

https://godbolt.org/z/e1zG4K7M8

This is not relevant for the kernel because -O2 is our default
optimization level (I'm not even sure that -O0 is buildable).
But you may want to mention that in commit message.

Thanks,
Yury