drivers/dma/hsu/hsu.c | 45 ++++++++++++------------------------------- drivers/dma/hsu/hsu.h | 4 ++-- 2 files changed, 14 insertions(+), 35 deletions(-)
Simplifies allocations by using a flexible array member in this struct.
Remove hsu_dma_alloc_desc(). It now offers no readability advantages in
this single usage.
Add __counted_by to get extra runtime analysis.
Apply the exact same treatment to struct hsu_dma and devm_kzalloc.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
v2: address review comments.
drivers/dma/hsu/hsu.c | 45 ++++++++++++-------------------------------
drivers/dma/hsu/hsu.h | 4 ++--
2 files changed, 14 insertions(+), 35 deletions(-)
diff --git a/drivers/dma/hsu/hsu.c b/drivers/dma/hsu/hsu.c
index f62d60d7bc6b..78a2352ada8c 100644
--- a/drivers/dma/hsu/hsu.c
+++ b/drivers/dma/hsu/hsu.c
@@ -241,28 +241,10 @@ int hsu_dma_do_irq(struct hsu_dma_chip *chip, unsigned short nr, u32 status)
}
EXPORT_SYMBOL_GPL(hsu_dma_do_irq);
-static struct hsu_dma_desc *hsu_dma_alloc_desc(unsigned int nents)
-{
- struct hsu_dma_desc *desc;
-
- desc = kzalloc_obj(*desc, GFP_NOWAIT);
- if (!desc)
- return NULL;
-
- desc->sg = kzalloc_objs(*desc->sg, nents, GFP_NOWAIT);
- if (!desc->sg) {
- kfree(desc);
- return NULL;
- }
-
- return desc;
-}
-
static void hsu_dma_desc_free(struct virt_dma_desc *vdesc)
{
struct hsu_dma_desc *desc = to_hsu_dma_desc(vdesc);
- kfree(desc->sg);
kfree(desc);
}
@@ -276,10 +258,15 @@ static struct dma_async_tx_descriptor *hsu_dma_prep_slave_sg(
struct scatterlist *sg;
unsigned int i;
- desc = hsu_dma_alloc_desc(sg_len);
+ desc = kzalloc_flex(*desc, sg, sg_len, GFP_NOWAIT);
if (!desc)
return NULL;
+ desc->nents = sg_len;
+ desc->direction = direction;
+ /* desc->active = 0 by kzalloc */
+ desc->status = DMA_IN_PROGRESS;
+
for_each_sg(sgl, sg, sg_len, i) {
desc->sg[i].addr = sg_dma_address(sg);
desc->sg[i].len = sg_dma_len(sg);
@@ -287,11 +274,6 @@ static struct dma_async_tx_descriptor *hsu_dma_prep_slave_sg(
desc->length += sg_dma_len(sg);
}
- desc->nents = sg_len;
- desc->direction = direction;
- /* desc->active = 0 by kzalloc */
- desc->status = DMA_IN_PROGRESS;
-
return vchan_tx_prep(&hsuc->vchan, &desc->vdesc, flags);
}
@@ -428,22 +410,19 @@ int hsu_dma_probe(struct hsu_dma_chip *chip)
{
struct hsu_dma *hsu;
void __iomem *addr = chip->regs + chip->offset;
+ unsigned short nr_channels;
unsigned short i;
int ret;
- 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;
INIT_LIST_HEAD(&hsu->dma.channels);
for (i = 0; i < hsu->nr_channels; i++) {
diff --git a/drivers/dma/hsu/hsu.h b/drivers/dma/hsu/hsu.h
index 3bca577b98a1..f6ca1014bccf 100644
--- a/drivers/dma/hsu/hsu.h
+++ b/drivers/dma/hsu/hsu.h
@@ -71,11 +71,11 @@ struct hsu_dma_sg {
struct hsu_dma_desc {
struct virt_dma_desc vdesc;
enum dma_transfer_direction direction;
- struct hsu_dma_sg *sg;
unsigned int nents;
size_t length;
unsigned int active;
enum dma_status status;
+ struct hsu_dma_sg sg[] __counted_by(nents);
};
static inline struct hsu_dma_desc *to_hsu_dma_desc(struct virt_dma_desc *vdesc)
@@ -115,8 +115,8 @@ struct hsu_dma {
struct dma_device dma;
/* channels */
- struct hsu_dma_chan *chan;
unsigned short nr_channels;
+ struct hsu_dma_chan chan[] __counted_by(nr_channels);
};
static inline struct hsu_dma *to_hsu_dma(struct dma_device *ddev)
--
2.53.0On Sat, Mar 28, 2026 at 9:17 PM Rosen Penev <rosenp@gmail.com> wrote: > > Simplifies allocations by using a flexible array member in this struct. > > Remove hsu_dma_alloc_desc(). It now offers no readability advantages in > this single usage. > > Add __counted_by to get extra runtime analysis. > Apply the exact same treatment to struct hsu_dma and devm_kzalloc. We refer to the functions as func(): devm_kzalloc(). ... > - 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? -- With Best Regards, Andy Shevchenko
On Mon, Mar 30, 2026 at 1:46 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Sat, Mar 28, 2026 at 9:17 PM Rosen Penev <rosenp@gmail.com> wrote: > > > > Simplifies allocations by using a flexible array member in this struct. > > > > Remove hsu_dma_alloc_desc(). It now offers no readability advantages in > > this single usage. > > > > Add __counted_by to get extra runtime analysis. > > > Apply the exact same treatment to struct hsu_dma and devm_kzalloc. > > We refer to the functions as func(): devm_kzalloc(). > > ... > > > - 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. > > -- > With Best Regards, > Andy Shevchenko
On Mon, Mar 30, 2026 at 11:41 PM Rosen Penev <rosenp@gmail.com> wrote: > On Mon, Mar 30, 2026 at 1:46 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Sat, Mar 28, 2026 at 9:17 PM Rosen Penev <rosenp@gmail.com> 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. 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 -- With Best Regards, Andy Shevchenko
On Mon, Mar 30, 2026 at 9:29 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Mar 30, 2026 at 11:41 PM Rosen Penev <rosenp@gmail.com> wrote: > > On Mon, Mar 30, 2026 at 1:46 AM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > On Sat, Mar 28, 2026 at 9:17 PM Rosen Penev <rosenp@gmail.com> 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? 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. > > 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 > > -- > With Best Regards, > Andy Shevchenko
On Wed, Apr 1, 2026 at 12:31 AM Rosen Penev <rosenp@gmail.com> wrote: > On Mon, Mar 30, 2026 at 9:29 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Mon, Mar 30, 2026 at 11:41 PM Rosen Penev <rosenp@gmail.com> wrote: > > > On Mon, Mar 30, 2026 at 1:46 AM Andy Shevchenko > > > <andy.shevchenko@gmail.com> wrote: > > > > On Sat, Mar 28, 2026 at 9:17 PM Rosen Penev <rosenp@gmail.com> 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 -- With Best Regards, Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.