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 <[email protected]>
---
  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))

Reply via email to