[PATCH] firmware: stratix10-svc: kmalloc_array + kzalloc to flex

[PATCH] firmware: stratix10-svc: kmalloc_array + kzalloc to flex

[Dinh Nguyen]

From: Rosen Penev <rosenp@gmail.com>

Use a flexible array member to combine allocations. Simplifies memory
management.

Add __counted_by for extra runtime analysis. Also move counting variable
assignment up as required by __counted_by.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 drivers/firmware/stratix10-svc.c | 82 ++++++++++++++------------------
 1 file changed, 37 insertions(+), 45 deletions(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index e9e35d67ef96..5a76cf3fc83a 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -240,10 +240,34 @@ struct stratix10_async_ctrl {
 	DECLARE_HASHTABLE(trx_list, ASYNC_TRX_HASH_BITS);
 };
 
+/**
+ * struct stratix10_svc_chan - service communication channel
+ * @ctrl: pointer to service controller which is the provider of this channel
+ * @scl: pointer to service client which owns the channel
+ * @name: service client name associated with the channel
+ * @task: pointer to the thread task which handles SMC or HVC call
+ * @svc_fifo: a queue for storing service message data (separate fifo for every channel)
+ * @svc_fifo_lock: protect access to service message data queue (locking pending fifo)
+ * @lock: protect access to the channel
+ * @async_chan: reference to asynchronous channel object for this channel
+ *
+ * This struct is used by service client to communicate with service layer.
+ * Each service client has its own channel created by service controller.
+ */
+struct stratix10_svc_chan {
+	struct stratix10_svc_controller *ctrl;
+	struct stratix10_svc_client *scl;
+	char *name;
+	struct task_struct *task;
+	struct kfifo svc_fifo;
+	spinlock_t svc_fifo_lock;
+	spinlock_t lock;
+	struct stratix10_async_chan *async_chan;
+};
+
 /**
  * struct stratix10_svc_controller - service controller
  * @dev: device
- * @chans: array of service channels
  * @num_chans: number of channels in 'chans' array
  * @num_active_client: number of active service client
  * @node: list management
@@ -253,13 +277,13 @@ struct stratix10_async_ctrl {
  * @svc: manages the list of client svc drivers
  * @sdm_lock: only allows a single command single response to SDM
  * @actrl: async control structure
+ * @chans: array of service channels
  *
  * This struct is used to create communication channels for service clients, to
  * handle secure monitor or hypervisor call.
  */
 struct stratix10_svc_controller {
 	struct device *dev;
-	struct stratix10_svc_chan *chans;
 	int num_chans;
 	int num_active_client;
 	struct list_head node;
@@ -269,31 +293,7 @@ struct stratix10_svc_controller {
 	struct stratix10_svc *svc;
 	struct mutex sdm_lock;
 	struct stratix10_async_ctrl actrl;
-};
-
-/**
- * struct stratix10_svc_chan - service communication channel
- * @ctrl: pointer to service controller which is the provider of this channel
- * @scl: pointer to service client which owns the channel
- * @name: service client name associated with the channel
- * @task: pointer to the thread task which handles SMC or HVC call
- * @svc_fifo: a queue for storing service message data (separate fifo for every channel)
- * @svc_fifo_lock: protect access to service message data queue (locking pending fifo)
- * @lock: protect access to the channel
- * @async_chan: reference to asynchronous channel object for this channel
- *
- * This struct is used by service client to communicate with service layer.
- * Each service client has its own channel created by service controller.
- */
-struct stratix10_svc_chan {
-	struct stratix10_svc_controller *ctrl;
-	struct stratix10_svc_client *scl;
-	char *name;
-	struct task_struct *task;
-	struct kfifo svc_fifo;
-	spinlock_t svc_fifo_lock;
-	spinlock_t lock;
-	struct stratix10_async_chan *async_chan;
+	struct stratix10_svc_chan chans[] __counted_by(num_chans);
 };
 
 static LIST_HEAD(svc_ctrl);
@@ -1901,7 +1901,6 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct stratix10_svc_controller *controller;
-	struct stratix10_svc_chan *chans;
 	struct gen_pool *genpool;
 	struct stratix10_svc_sh_memory *sh_memory;
 	struct stratix10_svc *svc = NULL;
@@ -1929,23 +1928,16 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 		return PTR_ERR(genpool);
 
 	/* allocate service controller and supporting channel */
-	controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
+	controller = devm_kzalloc(dev, struct_size(controller, chans, SVC_NUM_CHANNEL),
+				GFP_KERNEL);
 	if (!controller) {
 		ret = -ENOMEM;
 		goto err_destroy_pool;
 	}
 
-	chans = devm_kmalloc_array(dev, SVC_NUM_CHANNEL,
-				   sizeof(*chans), GFP_KERNEL | __GFP_ZERO);
-	if (!chans) {
-		ret = -ENOMEM;
-		goto err_destroy_pool;
-	}
-
-	controller->dev = dev;
 	controller->num_chans = SVC_NUM_CHANNEL;
+	controller->dev = dev;
 	controller->num_active_client = 0;
-	controller->chans = chans;
 	controller->genpool = genpool;
 	controller->invoke_fn = invoke_fn;
 	INIT_LIST_HEAD(&controller->node);
@@ -1962,16 +1954,16 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 	mutex_init(&controller->sdm_lock);
 
 	for (i = 0; i < SVC_NUM_CHANNEL; i++) {
-		chans[i].scl = NULL;
-		chans[i].ctrl = controller;
-		chans[i].name = (char *)chan_names[i];
-		spin_lock_init(&chans[i].lock);
-		ret = kfifo_alloc(&chans[i].svc_fifo, fifo_size, GFP_KERNEL);
+		controller->chans[i].scl = NULL;
+		controller->chans[i].ctrl = controller;
+		controller->chans[i].name = (char *)chan_names[i];
+		spin_lock_init(&controller->chans[i].lock);
+		ret = kfifo_alloc(&controller->chans[i].svc_fifo, fifo_size, GFP_KERNEL);
 		if (ret) {
 			dev_err(dev, "failed to allocate FIFO %d\n", i);
 			goto err_free_fifos;
 		}
-		spin_lock_init(&chans[i].svc_fifo_lock);
+		spin_lock_init(&controller->chans[i].svc_fifo_lock);
 	}
 
 	list_add_tail(&controller->node, &svc_ctrl);
@@ -2015,7 +2007,7 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 		list_del(&controller->node);
 	/* free only the FIFOs that were successfully allocated */
 	while (i--)
-		kfifo_free(&chans[i].svc_fifo);
+		kfifo_free(&controller->chans[i].svc_fifo);
 	stratix10_svc_async_exit(controller);
 err_destroy_pool:
 	gen_pool_destroy(genpool);
-- 
2.42.0.411.g813d9a9188

Re: [PATCH] firmware: stratix10-svc: kmalloc_array + kzalloc to flex

[Dinh Nguyen]

Hi Greg,

Gentle ping.

Thanks,
Dinh

On 3/30/26 06:51, Dinh Nguyen wrote:
> From: Rosen Penev <rosenp@gmail.com>
> 
> Use a flexible array member to combine allocations. Simplifies memory
> management.
> 
> Add __counted_by for extra runtime analysis. Also move counting variable
> assignment up as required by __counted_by.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>   drivers/firmware/stratix10-svc.c | 82 ++++++++++++++------------------
>   1 file changed, 37 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index e9e35d67ef96..5a76cf3fc83a 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -240,10 +240,34 @@ struct stratix10_async_ctrl {
>   	DECLARE_HASHTABLE(trx_list, ASYNC_TRX_HASH_BITS);
>   };
>   
> +/**
> + * struct stratix10_svc_chan - service communication channel
> + * @ctrl: pointer to service controller which is the provider of this channel
> + * @scl: pointer to service client which owns the channel
> + * @name: service client name associated with the channel
> + * @task: pointer to the thread task which handles SMC or HVC call
> + * @svc_fifo: a queue for storing service message data (separate fifo for every channel)
> + * @svc_fifo_lock: protect access to service message data queue (locking pending fifo)
> + * @lock: protect access to the channel
> + * @async_chan: reference to asynchronous channel object for this channel
> + *
> + * This struct is used by service client to communicate with service layer.
> + * Each service client has its own channel created by service controller.
> + */
> +struct stratix10_svc_chan {
> +	struct stratix10_svc_controller *ctrl;
> +	struct stratix10_svc_client *scl;
> +	char *name;
> +	struct task_struct *task;
> +	struct kfifo svc_fifo;
> +	spinlock_t svc_fifo_lock;
> +	spinlock_t lock;
> +	struct stratix10_async_chan *async_chan;
> +};
> +
>   /**
>    * struct stratix10_svc_controller - service controller
>    * @dev: device
> - * @chans: array of service channels
>    * @num_chans: number of channels in 'chans' array
>    * @num_active_client: number of active service client
>    * @node: list management
> @@ -253,13 +277,13 @@ struct stratix10_async_ctrl {
>    * @svc: manages the list of client svc drivers
>    * @sdm_lock: only allows a single command single response to SDM
>    * @actrl: async control structure
> + * @chans: array of service channels
>    *
>    * This struct is used to create communication channels for service clients, to
>    * handle secure monitor or hypervisor call.
>    */
>   struct stratix10_svc_controller {
>   	struct device *dev;
> -	struct stratix10_svc_chan *chans;
>   	int num_chans;
>   	int num_active_client;
>   	struct list_head node;
> @@ -269,31 +293,7 @@ struct stratix10_svc_controller {
>   	struct stratix10_svc *svc;
>   	struct mutex sdm_lock;
>   	struct stratix10_async_ctrl actrl;
> -};
> -
> -/**
> - * struct stratix10_svc_chan - service communication channel
> - * @ctrl: pointer to service controller which is the provider of this channel
> - * @scl: pointer to service client which owns the channel
> - * @name: service client name associated with the channel
> - * @task: pointer to the thread task which handles SMC or HVC call
> - * @svc_fifo: a queue for storing service message data (separate fifo for every channel)
> - * @svc_fifo_lock: protect access to service message data queue (locking pending fifo)
> - * @lock: protect access to the channel
> - * @async_chan: reference to asynchronous channel object for this channel
> - *
> - * This struct is used by service client to communicate with service layer.
> - * Each service client has its own channel created by service controller.
> - */
> -struct stratix10_svc_chan {
> -	struct stratix10_svc_controller *ctrl;
> -	struct stratix10_svc_client *scl;
> -	char *name;
> -	struct task_struct *task;
> -	struct kfifo svc_fifo;
> -	spinlock_t svc_fifo_lock;
> -	spinlock_t lock;
> -	struct stratix10_async_chan *async_chan;
> +	struct stratix10_svc_chan chans[] __counted_by(num_chans);
>   };
>   
>   static LIST_HEAD(svc_ctrl);
> @@ -1901,7 +1901,6 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct stratix10_svc_controller *controller;
> -	struct stratix10_svc_chan *chans;
>   	struct gen_pool *genpool;
>   	struct stratix10_svc_sh_memory *sh_memory;
>   	struct stratix10_svc *svc = NULL;
> @@ -1929,23 +1928,16 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
>   		return PTR_ERR(genpool);
>   
>   	/* allocate service controller and supporting channel */
> -	controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
> +	controller = devm_kzalloc(dev, struct_size(controller, chans, SVC_NUM_CHANNEL),
> +				GFP_KERNEL);
>   	if (!controller) {
>   		ret = -ENOMEM;
>   		goto err_destroy_pool;
>   	}
>   
> -	chans = devm_kmalloc_array(dev, SVC_NUM_CHANNEL,
> -				   sizeof(*chans), GFP_KERNEL | __GFP_ZERO);
> -	if (!chans) {
> -		ret = -ENOMEM;
> -		goto err_destroy_pool;
> -	}
> -
> -	controller->dev = dev;
>   	controller->num_chans = SVC_NUM_CHANNEL;
> +	controller->dev = dev;
>   	controller->num_active_client = 0;
> -	controller->chans = chans;
>   	controller->genpool = genpool;
>   	controller->invoke_fn = invoke_fn;
>   	INIT_LIST_HEAD(&controller->node);
> @@ -1962,16 +1954,16 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
>   	mutex_init(&controller->sdm_lock);
>   
>   	for (i = 0; i < SVC_NUM_CHANNEL; i++) {
> -		chans[i].scl = NULL;
> -		chans[i].ctrl = controller;
> -		chans[i].name = (char *)chan_names[i];
> -		spin_lock_init(&chans[i].lock);
> -		ret = kfifo_alloc(&chans[i].svc_fifo, fifo_size, GFP_KERNEL);
> +		controller->chans[i].scl = NULL;
> +		controller->chans[i].ctrl = controller;
> +		controller->chans[i].name = (char *)chan_names[i];
> +		spin_lock_init(&controller->chans[i].lock);
> +		ret = kfifo_alloc(&controller->chans[i].svc_fifo, fifo_size, GFP_KERNEL);
>   		if (ret) {
>   			dev_err(dev, "failed to allocate FIFO %d\n", i);
>   			goto err_free_fifos;
>   		}
> -		spin_lock_init(&chans[i].svc_fifo_lock);
> +		spin_lock_init(&controller->chans[i].svc_fifo_lock);
>   	}
>   
>   	list_add_tail(&controller->node, &svc_ctrl);
> @@ -2015,7 +2007,7 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
>   		list_del(&controller->node);
>   	/* free only the FIFOs that were successfully allocated */
>   	while (i--)
> -		kfifo_free(&chans[i].svc_fifo);
> +		kfifo_free(&controller->chans[i].svc_fifo);
>   	stratix10_svc_async_exit(controller);
>   err_destroy_pool:
>   	gen_pool_destroy(genpool);

[PATCH] firmware: stratix10-svc: kmalloc_array + kzalloc to flex

[Rosen Penev]

Use a flexible array member to combine allocations. Simplifies memory
management.

Add __counted_by for extra runtime analysis. Also move counting variable
assignment up as required by __counted_by.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/firmware/stratix10-svc.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index e9e35d67ef96..e28adbc25eb4 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -243,7 +243,6 @@ struct stratix10_async_ctrl {
 /**
  * struct stratix10_svc_controller - service controller
  * @dev: device
- * @chans: array of service channels
  * @num_chans: number of channels in 'chans' array
  * @num_active_client: number of active service client
  * @node: list management
@@ -253,13 +252,13 @@ struct stratix10_async_ctrl {
  * @svc: manages the list of client svc drivers
  * @sdm_lock: only allows a single command single response to SDM
  * @actrl: async control structure
+ * @chans: array of service channels
  *
  * This struct is used to create communication channels for service clients, to
  * handle secure monitor or hypervisor call.
  */
 struct stratix10_svc_controller {
 	struct device *dev;
-	struct stratix10_svc_chan *chans;
 	int num_chans;
 	int num_active_client;
 	struct list_head node;
@@ -269,6 +268,7 @@ struct stratix10_svc_controller {
 	struct stratix10_svc *svc;
 	struct mutex sdm_lock;
 	struct stratix10_async_ctrl actrl;
+	struct stratix10_svc_chan chans[] __counted_by(num_chans);
 };
 
 /**
@@ -1901,7 +1901,6 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct stratix10_svc_controller *controller;
-	struct stratix10_svc_chan *chans;
 	struct gen_pool *genpool;
 	struct stratix10_svc_sh_memory *sh_memory;
 	struct stratix10_svc *svc = NULL;
@@ -1929,23 +1928,16 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 		return PTR_ERR(genpool);
 
 	/* allocate service controller and supporting channel */
-	controller = devm_kzalloc(dev, sizeof(*controller), GFP_KERNEL);
+	controller = devm_kzalloc(dev, struct_size(controller, chans, SVC_NUM_CHANNEL),
+				GFP_KERNEL);
 	if (!controller) {
 		ret = -ENOMEM;
 		goto err_destroy_pool;
 	}
 
-	chans = devm_kmalloc_array(dev, SVC_NUM_CHANNEL,
-				   sizeof(*chans), GFP_KERNEL | __GFP_ZERO);
-	if (!chans) {
-		ret = -ENOMEM;
-		goto err_destroy_pool;
-	}
-
-	controller->dev = dev;
 	controller->num_chans = SVC_NUM_CHANNEL;
+	controller->dev = dev;
 	controller->num_active_client = 0;
-	controller->chans = chans;
 	controller->genpool = genpool;
 	controller->invoke_fn = invoke_fn;
 	INIT_LIST_HEAD(&controller->node);
@@ -1962,16 +1954,16 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 	mutex_init(&controller->sdm_lock);
 
 	for (i = 0; i < SVC_NUM_CHANNEL; i++) {
-		chans[i].scl = NULL;
-		chans[i].ctrl = controller;
-		chans[i].name = (char *)chan_names[i];
-		spin_lock_init(&chans[i].lock);
-		ret = kfifo_alloc(&chans[i].svc_fifo, fifo_size, GFP_KERNEL);
+		controller->chans[i].scl = NULL;
+		controller->chans[i].ctrl = controller;
+		controller->chans[i].name = (char *)chan_names[i];
+		spin_lock_init(&controller->chans[i].lock);
+		ret = kfifo_alloc(&controller->chans[i].svc_fifo, fifo_size, GFP_KERNEL);
 		if (ret) {
 			dev_err(dev, "failed to allocate FIFO %d\n", i);
 			goto err_free_fifos;
 		}
-		spin_lock_init(&chans[i].svc_fifo_lock);
+		spin_lock_init(&controller->chans[i].svc_fifo_lock);
 	}
 
 	list_add_tail(&controller->node, &svc_ctrl);
@@ -2015,7 +2007,7 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
 		list_del(&controller->node);
 	/* free only the FIFOs that were successfully allocated */
 	while (i--)
-		kfifo_free(&chans[i].svc_fifo);
+		kfifo_free(&controller->chans[i].svc_fifo);
 	stratix10_svc_async_exit(controller);
 err_destroy_pool:
 	gen_pool_destroy(genpool);
-- 
2.53.0