On Wed, Apr 01, 2026 at 04:32:13PM +0300, Andy Shevchenko wrote: > On Wed, Apr 1, 2026 at 12:31 AM Rosen Penev <[email protected]> wrote: > > On Mon, Mar 30, 2026 at 9:29 PM Andy Shevchenko > > <[email protected]> wrote: > > > On Mon, Mar 30, 2026 at 11:41 PM Rosen Penev <[email protected]> wrote: > > > > On Mon, Mar 30, 2026 at 1:46 AM Andy Shevchenko > > > > <[email protected]> wrote: > > > > > On Sat, Mar 28, 2026 at 9:17 PM Rosen Penev <[email protected]> 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
