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

[Andy Shevchenko]

On Tue, Apr 14, 2026 at 11:47 PM Rosen Penev <rosenp@xxxxxxxxx> wrote:
> On Tue, Apr 14, 2026 at 5:41 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxx> wrote:
> > 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.
> Merge window is closed right now AFAIK.

You mean opened. In any case this kind of patch can still be sent,
it's just no time to review by the maintainers, but on the positive
side while they are busy with other stuff all kind of CIs, test bots,
AI reviews can be performed before the maintainer has a chance to look
at this. Which saves their time as well.


-- 
With Best Regards,
Andy Shevchenko