* [PATCH] accel/qaic: kcalloc + kzalloc to kzalloc
@ 2026-03-17 0:30 Rosen Penev
2026-03-27 16:16 ` Jeff Hugo
0 siblings, 1 reply; 2+ messages in thread
From: Rosen Penev @ 2026-03-17 0:30 UTC (permalink / raw)
To: linux-kernel
Cc: Jeff Hugo, Carl Vanderlip, Oded Gabbay, Kees Cook,
Gustavo A. R. Silva, open list:QUALCOMM CLOUD AI (QAIC) DRIVER,
open list:QUALCOMM CLOUD AI (QAIC) DRIVER,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
Combine allocations by using a flexible array member.
Use __counted_by for extra runtime analysis. Move counting variable
assignment as required by __counted_by.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/accel/qaic/qaic.h | 4 ++--
drivers/accel/qaic/qaic_drv.c | 7 ++-----
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index fa7a8155658c..e237020f6aa9 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -152,8 +152,6 @@ struct qaic_device {
struct list_head cntl_xfer_list;
/* Synchronizes MHI control device transactions and its xfer list */
struct mutex cntl_mutex;
- /* Array of DBC struct of this device */
- struct dma_bridge_chan *dbc;
/* Work queue for tasks related to MHI control device */
struct workqueue_struct *cntl_wq;
/* Synchronizes all the users of device during cleanup */
@@ -206,6 +204,8 @@ struct qaic_device {
void *ssr_mhi_buf;
/* DBC which is under SSR. Sentinel U32_MAX would mean that no SSR in progress */
u32 ssr_dbc;
+ /* Array of DBC struct of this device */
+ struct dma_bridge_chan dbc[] __counted_by(num_dbc);
};
struct qaic_drm_device {
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index 63fb8c7b4abc..ab428ecd26f5 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -405,15 +405,12 @@ static struct qaic_device *create_qdev(struct pci_dev *pdev,
struct drm_device *drm;
int i, ret;
- qdev = devm_kzalloc(dev, sizeof(*qdev), GFP_KERNEL);
+ qdev = devm_kzalloc(dev, struct_size(qdev, dbc, 16), GFP_KERNEL);
if (!qdev)
return NULL;
- qdev->dev_state = QAIC_OFFLINE;
qdev->num_dbc = 16;
- qdev->dbc = devm_kcalloc(dev, qdev->num_dbc, sizeof(*qdev->dbc), GFP_KERNEL);
- if (!qdev->dbc)
- return NULL;
+ qdev->dev_state = QAIC_OFFLINE;
qddev = devm_drm_dev_alloc(&pdev->dev, &qaic_accel_driver, struct qaic_drm_device, drm);
if (IS_ERR(qddev))
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] accel/qaic: kcalloc + kzalloc to kzalloc
2026-03-17 0:30 [PATCH] accel/qaic: kcalloc + kzalloc to kzalloc Rosen Penev
@ 2026-03-27 16:16 ` Jeff Hugo
0 siblings, 0 replies; 2+ messages in thread
From: Jeff Hugo @ 2026-03-27 16:16 UTC (permalink / raw)
To: Rosen Penev, linux-kernel
Cc: Carl Vanderlip, Oded Gabbay, Kees Cook, Gustavo A. R. Silva,
open list:QUALCOMM CLOUD AI (QAIC) DRIVER,
open list:QUALCOMM CLOUD AI (QAIC) DRIVER,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
On 3/16/2026 6:30 PM, Rosen Penev wrote:
> Combine allocations by using a flexible array member.
>
> Use __counted_by for extra runtime analysis. Move counting variable
> assignment as required by __counted_by.
This fails to answer "why". This change is not justified in the commit
text here. Please see "Describe your changes" in submitting-patches.
Also would be helpful for you to mention how this was tested (although
those details don't need to be in the commit text itself). You are
touching arguably the core structure of the driver and any mistakes will
be devastating.
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/accel/qaic/qaic.h | 4 ++--
> drivers/accel/qaic/qaic_drv.c | 7 ++-----
> 2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
> index fa7a8155658c..e237020f6aa9 100644
> --- a/drivers/accel/qaic/qaic.h
> +++ b/drivers/accel/qaic/qaic.h
> @@ -152,8 +152,6 @@ struct qaic_device {
> struct list_head cntl_xfer_list;
> /* Synchronizes MHI control device transactions and its xfer list */
> struct mutex cntl_mutex;
> - /* Array of DBC struct of this device */
> - struct dma_bridge_chan *dbc;
> /* Work queue for tasks related to MHI control device */
> struct workqueue_struct *cntl_wq;
> /* Synchronizes all the users of device during cleanup */
> @@ -206,6 +204,8 @@ struct qaic_device {
> void *ssr_mhi_buf;
> /* DBC which is under SSR. Sentinel U32_MAX would mean that no SSR in progress */
> u32 ssr_dbc;
> + /* Array of DBC struct of this device */
> + struct dma_bridge_chan dbc[] __counted_by(num_dbc);
> };
>
> struct qaic_drm_device {
> diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
> index 63fb8c7b4abc..ab428ecd26f5 100644
> --- a/drivers/accel/qaic/qaic_drv.c
> +++ b/drivers/accel/qaic/qaic_drv.c
> @@ -405,15 +405,12 @@ static struct qaic_device *create_qdev(struct pci_dev *pdev,
> struct drm_device *drm;
> int i, ret;
>
> - qdev = devm_kzalloc(dev, sizeof(*qdev), GFP_KERNEL);
> + qdev = devm_kzalloc(dev, struct_size(qdev, dbc, 16), GFP_KERNEL);
I dislike the structure of this change. 16 is now in multiple places,
and when it will change in the near future, its going to be harder to
get the update right. The existing code specifically assigns the value
and uses the value from that assignment for that reason.
> if (!qdev)
> return NULL;
>
> - qdev->dev_state = QAIC_OFFLINE;
> qdev->num_dbc = 16;
> - qdev->dbc = devm_kcalloc(dev, qdev->num_dbc, sizeof(*qdev->dbc), GFP_KERNEL);
> - if (!qdev->dbc)
> - return NULL;
> + qdev->dev_state = QAIC_OFFLINE;
>
> qddev = devm_drm_dev_alloc(&pdev->dev, &qaic_accel_driver, struct qaic_drm_device, drm);
> if (IS_ERR(qddev))
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-03-27 16:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-17 0:30 [PATCH] accel/qaic: kcalloc + kzalloc to kzalloc Rosen Penev
2026-03-27 16:16 ` Jeff Hugo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox