[PATCH] atm: nicstar: simplify allocation

[PATCH] atm: nicstar: simplify allocation

[Rosen Penev]

Use a flexible array member with kzalloc_flex to combine allocations
into one.

Add __counted_by for extra runtime analysis. Move counting variable
assignment to right after allocation as required by __counted_by.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/atm/nicstar.c | 17 +++++------------
 drivers/atm/nicstar.h |  4 ++--
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
index 24e51343df15..2f6a3637aeab 100644
--- a/drivers/atm/nicstar.c
+++ b/drivers/atm/nicstar.c
@@ -867,23 +867,17 @@ static scq_info *get_scq(ns_dev *card, int size, u32 scd)
 	if (size != VBR_SCQSIZE && size != CBR_SCQSIZE)
 		return NULL;
 
-	scq = kmalloc_obj(*scq);
+	scq = kmalloc_flex(*scq, skb, size / NS_SCQE_SIZE);
 	if (!scq)
 		return NULL;
-        scq->org = dma_alloc_coherent(&card->pcidev->dev,
-				      2 * size,  &scq->dma, GFP_KERNEL);
+
+	scq->num_entries = size / NS_SCQE_SIZE;
+
+	scq->org = dma_alloc_coherent(&card->pcidev->dev, 2 * size, &scq->dma, GFP_KERNEL);
 	if (!scq->org) {
 		kfree(scq);
 		return NULL;
 	}
-	scq->skb = kzalloc_objs(*scq->skb, size / NS_SCQE_SIZE);
-	if (!scq->skb) {
-		dma_free_coherent(&card->pcidev->dev,
-				  2 * size, scq->org, scq->dma);
-		kfree(scq);
-		return NULL;
-	}
-	scq->num_entries = size / NS_SCQE_SIZE;
 	scq->base = PTR_ALIGN(scq->org, size);
 	scq->next = scq->base;
 	scq->last = scq->base + (scq->num_entries - 1);
@@ -928,7 +922,6 @@ static void free_scq(ns_dev *card, scq_info *scq, struct atm_vcc *vcc)
 				}
 			}
 	}
-	kfree(scq->skb);
 	dma_free_coherent(&card->pcidev->dev,
 			  2 * (scq->num_entries == VBR_SCQ_NUM_ENTRIES ?
 			       VBR_SCQSIZE : CBR_SCQSIZE),
diff --git a/drivers/atm/nicstar.h b/drivers/atm/nicstar.h
index 1b7f1dfc1735..ba45be53d40e 100644
--- a/drivers/atm/nicstar.h
+++ b/drivers/atm/nicstar.h
@@ -667,14 +667,14 @@ typedef struct scq_info {
 	ns_scqe *next;
 	volatile ns_scqe *tail;	/* Not related to the nicstar register */
 	unsigned num_entries;
-	struct sk_buff **skb;	/* Pointer to an array of pointers
-				   to the sk_buffs used for tx */
 	u32 scd;		/* SRAM address of the corresponding
 				   SCD */
 	int tbd_count;		/* Only meaningful on variable rate */
 	wait_queue_head_t scqfull_waitq;
 	volatile char full;	/* SCQ full indicator */
 	spinlock_t lock;	/* SCQ spinlock */
+	struct sk_buff *skb[] __counted_by(num_entries);	/* Pointer to an array of pointers
+								   to the sk_buffs used for tx */
 } scq_info;
 
 typedef struct rsq_info {
-- 
2.53.0

Re: [PATCH] atm: nicstar: simplify allocation

[Simon Horman]

On Mon, Mar 23, 2026 at 08:22:43PM -0700, Rosen Penev wrote:
> Use a flexible array member with kzalloc_flex to combine allocations
> into one.
> 
> Add __counted_by for extra runtime analysis. Move counting variable
> assignment to right after allocation as required by __counted_by.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

To be honest I'd prefer that we discussed deprecating and removing ATM
rather than cleaning it up. Is anyone using it?

> ---
>  drivers/atm/nicstar.c | 17 +++++------------
>  drivers/atm/nicstar.h |  4 ++--
>  2 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
> index 24e51343df15..2f6a3637aeab 100644
> --- a/drivers/atm/nicstar.c
> +++ b/drivers/atm/nicstar.c
> @@ -867,23 +867,17 @@ static scq_info *get_scq(ns_dev *card, int size, u32 scd)
>  	if (size != VBR_SCQSIZE && size != CBR_SCQSIZE)
>  		return NULL;
>  
> -	scq = kmalloc_obj(*scq);
> +	scq = kmalloc_flex(*scq, skb, size / NS_SCQE_SIZE);
>  	if (!scq)
>  		return NULL;
> -        scq->org = dma_alloc_coherent(&card->pcidev->dev,
> -				      2 * size,  &scq->dma, GFP_KERNEL);
> +
> +	scq->num_entries = size / NS_SCQE_SIZE;
> +
> +	scq->org = dma_alloc_coherent(&card->pcidev->dev, 2 * size, &scq->dma, GFP_KERNEL);
>  	if (!scq->org) {
>  		kfree(scq);
>  		return NULL;
>  	}
> -	scq->skb = kzalloc_objs(*scq->skb, size / NS_SCQE_SIZE);
> -	if (!scq->skb) {
> -		dma_free_coherent(&card->pcidev->dev,
> -				  2 * size, scq->org, scq->dma);
> -		kfree(scq);
> -		return NULL;
> -	}
> -	scq->num_entries = size / NS_SCQE_SIZE;

AI review points out that previously the skb array was zeroed on allocation,
which is relied on by the code. But that is no longer the case.

-- 
pw-bot: changes requested

Re: [PATCH] atm: nicstar: simplify allocation

[Rosen Penev]

On Wed, Mar 25, 2026 at 10:27 AM Simon Horman <horms@kernel.org> wrote:
>
> On Mon, Mar 23, 2026 at 08:22:43PM -0700, Rosen Penev wrote:
> > Use a flexible array member with kzalloc_flex to combine allocations
> > into one.
> >
> > Add __counted_by for extra runtime analysis. Move counting variable
> > assignment to right after allocation as required by __counted_by.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
> To be honest I'd prefer that we discussed deprecating and removing ATM
> rather than cleaning it up. Is anyone using it?
Beats me.
>
> > ---
> >  drivers/atm/nicstar.c | 17 +++++------------
> >  drivers/atm/nicstar.h |  4 ++--
> >  2 files changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
> > index 24e51343df15..2f6a3637aeab 100644
> > --- a/drivers/atm/nicstar.c
> > +++ b/drivers/atm/nicstar.c
> > @@ -867,23 +867,17 @@ static scq_info *get_scq(ns_dev *card, int size, u32 scd)
> >       if (size != VBR_SCQSIZE && size != CBR_SCQSIZE)
> >               return NULL;
> >
> > -     scq = kmalloc_obj(*scq);
> > +     scq = kmalloc_flex(*scq, skb, size / NS_SCQE_SIZE);
> >       if (!scq)
> >               return NULL;
> > -        scq->org = dma_alloc_coherent(&card->pcidev->dev,
> > -                                   2 * size,  &scq->dma, GFP_KERNEL);
> > +
> > +     scq->num_entries = size / NS_SCQE_SIZE;
> > +
> > +     scq->org = dma_alloc_coherent(&card->pcidev->dev, 2 * size, &scq->dma, GFP_KERNEL);
> >       if (!scq->org) {
> >               kfree(scq);
> >               return NULL;
> >       }
> > -     scq->skb = kzalloc_objs(*scq->skb, size / NS_SCQE_SIZE);
> > -     if (!scq->skb) {
> > -             dma_free_coherent(&card->pcidev->dev,
> > -                               2 * size, scq->org, scq->dma);
> > -             kfree(scq);
> > -             return NULL;
> > -     }
> > -     scq->num_entries = size / NS_SCQE_SIZE;
>
> AI review points out that previously the skb array was zeroed on allocation,
> which is relied on by the code. But that is no longer the case.
Will fix.
>
> --
> pw-bot: changes requested