Re: [PATCHv2] dmaengine: hsu: use kzalloc_flex()

[Andy Shevchenko]

On Wed, Apr 01, 2026 at 04:32:13PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 1, 2026 at 12:31 AM Rosen Penev <rosenp@xxxxxxxxx> wrote:
> > On Mon, Mar 30, 2026 at 9:29 PM Andy Shevchenko
> > <andy.shevchenko@xxxxxxxxx> wrote:
> > > On Mon, Mar 30, 2026 at 11:41 PM Rosen Penev <rosenp@xxxxxxxxx> wrote:
> > > > On Mon, Mar 30, 2026 at 1:46 AM Andy Shevchenko
> > > > <andy.shevchenko@xxxxxxxxx> wrote:
> > > > > On Sat, Mar 28, 2026 at 9:17 PM Rosen Penev <rosenp@xxxxxxxxx> wrote:

...

> > > > > > -       hsu = devm_kzalloc(chip->dev, sizeof(*hsu), GFP_KERNEL);
> > > > > > +       /* Calculate nr_channels from the IO space length */
> > > > > > +       nr_channels = (chip->length - chip->offset) / HSU_DMA_CHAN_LENGTH;
> > > > > > +       hsu = devm_kzalloc(chip->dev, struct_size(hsu, chan, nr_channels), GFP_KERNEL);
> > > > > >         if (!hsu)
> > > > > >                 return -ENOMEM;
> > > > > >
> > > > > > -       chip->hsu = hsu;
> > > > > > -
> > > > > > -       /* Calculate nr_channels from the IO space length */
> > > > > > -       hsu->nr_channels = (chip->length - chip->offset) / HSU_DMA_CHAN_LENGTH;
> > > > > > +       hsu->nr_channels = nr_channels;
> > > > > >
> > > > > > -       hsu->chan = devm_kcalloc(chip->dev, hsu->nr_channels,
> > > > > > -                                sizeof(*hsu->chan), GFP_KERNEL);
> > > > > > -       if (!hsu->chan)
> > > > > > -               return -ENOMEM;
> > > > > > +       chip->hsu = hsu;
> > > > >
> > > > > Don't know these _flex() APIs enough, but can we leave the chip->hsu =
> > > > > hsu; in the same place as it's now?
> > > > __counted_by requires the first assignment after allocation to be the
> > > > counting variable. The _flex macros do this automatically for GCC15
> > > > and above.
> > >
> > > Why? The hsu member has nothing to do with VLA, where is this
> > > requirement coming from? My understanding is that the check should
> > > imply the minimum sizeof of the data structure and the compiler should
> > > know that way before doing any allocations.
> > Not sure I follow. This patch changes hsu's chan member to a FAM.
> > Where is VLA coming from?
> 
> VLA: variable-length array
> FAM: flexible array member
> The second one is VLA member + size member.
> 
> What your patch is doing is changing a pointer to VLA member.
> 
> > The current code is devm_kzalloc(x, struct_size()). When it gets
> > introduced, I'm sure there will be a treewide conversion to
> > devm_kzalloc_flex, which would automatically set the counting variable
> > for >=GCC15.
> >
> > It's best practice to assign right after since kzalloc_flex does it anyways.
> 
> Still, I'm not convinced we should blindly follow this rule. The
> length needs to be known before accessing the VLA, but it's okay to
> access other members. Leaving hsu member assignment where it's now is
> fine, no need to move it around.
> 
> > > My understanding seems in align with what Gustavo blogged:
> > > https://people.kernel.org/gustavoars/how-to-use-the-new-counted_by-attribute-in-c-and-linux
> > >
> > > The same is written in the GCC patch description
> > > https://gcc.gnu.org/pipermail/gcc-patches/2024-May/653123.html

If you agree with my reasoning, please send a v4, I will give you a tag.

Otherwise I really would like to understand the justification why the
assignment going first is the best practice and how it may help the developer.

-- 
With Best Regards,
Andy Shevchenko