dmaengine: idxd: fix wq conf_dev 'struct device' lifetime

Remove devm_* allocation and fix wq->conf_dev 'struct device' lifetime.
Address issues flagged by CONFIG_DEBUG_KOBJECT_RELEASE. Add release
functions in order to free the allocated memory for the wq context at
device destruction time.

Reported-by: Jason Gunthorpe <jgg@nvidia.com>
Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators")
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Link: https://lore.kernel.org/r/161852985907.2203940.6840120734115043753.stgit@djiang5-desk3.ch.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 3f696abd74acb229e33035e471b78cbb64fff526..c4183294a7041f8ef9b7f2dc441c3fdac0f05df6 100644 (file)
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -520,7 +520,7 @@ void idxd_device_wqs_clear_state(struct idxd_device *idxd)
        lockdep_assert_held(&idxd->dev_lock);
 
        for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
+               struct idxd_wq *wq = idxd->wqs[i];
 
                if (wq->state == IDXD_WQ_ENABLED) {
                        idxd_wq_disable_cleanup(wq);
@@ -738,7 +738,7 @@ static int idxd_wqs_config_write(struct idxd_device *idxd)
        int i, rc;
 
        for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
+               struct idxd_wq *wq = idxd->wqs[i];
 
                rc = idxd_wq_config_write(wq);
                if (rc < 0)
@@ -816,7 +816,7 @@ static int idxd_wqs_setup(struct idxd_device *idxd)
        }
 
        for (i = 0; i < idxd->max_wqs; i++) {
-               wq = &idxd->wqs[i];
+               wq = idxd->wqs[i];
                group = wq->group;
 
                if (!wq->group)

diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index bb3a580732afc0f6457b9ca384297777a479c527..6cade6a05314946678e1e39f96cd6ec64a376df5 100644 (file)
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -194,7 +194,7 @@ struct idxd_device {
        spinlock_t dev_lock;    /* spinlock for device */
        struct completion *cmd_done;
        struct idxd_group *groups;
-       struct idxd_wq *wqs;
+       struct idxd_wq **wqs;
        struct idxd_engine *engines;
 
        struct iommu_sva *sva;
@@ -258,6 +258,7 @@ extern struct bus_type iax_bus_type;
 extern bool support_enqcmd;
 extern struct device_type dsa_device_type;
 extern struct device_type iax_device_type;
+extern struct device_type idxd_wq_device_type;
 
 static inline bool is_dsa_dev(struct device *dev)
 {
@@ -274,6 +275,23 @@ static inline bool is_idxd_dev(struct device *dev)
        return is_dsa_dev(dev) || is_iax_dev(dev);
 }
 
+static inline bool is_idxd_wq_dev(struct device *dev)
+{
+       return dev->type == &idxd_wq_device_type;
+}
+
+static inline bool is_idxd_wq_dmaengine(struct idxd_wq *wq)
+{
+       if (wq->type == IDXD_WQT_KERNEL && strcmp(wq->name, "dmaengine") == 0)
+               return true;
+       return false;
+}
+
+static inline bool is_idxd_wq_cdev(struct idxd_wq *wq)
+{
+       return wq->type == IDXD_WQT_USER;
+}
+
 static inline bool wq_dedicated(struct idxd_wq *wq)
 {
        return test_bit(WQ_FLAG_DEDICATED, &wq->flags);

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 17d3b36610a907633c283ddae32bf1d12dc66e5d..a2dca27aebc3379f90e2863c75320c41a92b37a7 100644 (file)
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -145,16 +145,74 @@ static int idxd_setup_interrupts(struct idxd_device *idxd)
        return rc;
 }
 
+static int idxd_setup_wqs(struct idxd_device *idxd)
+{
+       struct device *dev = &idxd->pdev->dev;
+       struct idxd_wq *wq;
+       int i, rc;
+
+       idxd->wqs = kcalloc_node(idxd->max_wqs, sizeof(struct idxd_wq *),
+                                GFP_KERNEL, dev_to_node(dev));
+       if (!idxd->wqs)
+               return -ENOMEM;
+
+       for (i = 0; i < idxd->max_wqs; i++) {
+               wq = kzalloc_node(sizeof(*wq), GFP_KERNEL, dev_to_node(dev));
+               if (!wq) {
+                       rc = -ENOMEM;
+                       goto err;
+               }
+
+               wq->id = i;
+               wq->idxd = idxd;
+               device_initialize(&wq->conf_dev);
+               wq->conf_dev.parent = &idxd->conf_dev;
+               wq->conf_dev.bus = idxd_get_bus_type(idxd);
+               wq->conf_dev.type = &idxd_wq_device_type;
+               rc = dev_set_name(&wq->conf_dev, "wq%d.%d", idxd->id, wq->id);
+               if (rc < 0) {
+                       put_device(&wq->conf_dev);
+                       goto err;
+               }
+
+               mutex_init(&wq->wq_lock);
+               wq->idxd_cdev.minor = -1;
+               wq->max_xfer_bytes = idxd->max_xfer_bytes;
+               wq->max_batch_size = idxd->max_batch_size;
+               wq->wqcfg = kzalloc_node(idxd->wqcfg_size, GFP_KERNEL, dev_to_node(dev));
+               if (!wq->wqcfg) {
+                       put_device(&wq->conf_dev);
+                       rc = -ENOMEM;
+                       goto err;
+               }
+               idxd->wqs[i] = wq;
+       }
+
+       return 0;
+
+ err:
+       while (--i >= 0)
+               put_device(&idxd->wqs[i]->conf_dev);
+       return rc;
+}
+
 static int idxd_setup_internals(struct idxd_device *idxd)
 {
        struct device *dev = &idxd->pdev->dev;
-       int i;
+       int i, rc;
 
        init_waitqueue_head(&idxd->cmd_waitq);
+
+       rc = idxd_setup_wqs(idxd);
+       if (rc < 0)
+               return rc;
+
        idxd->groups = devm_kcalloc(dev, idxd->max_groups,
                                    sizeof(struct idxd_group), GFP_KERNEL);
-       if (!idxd->groups)
-               return -ENOMEM;
+       if (!idxd->groups) {
+               rc = -ENOMEM;
+               goto err;
+       }
 
        for (i = 0; i < idxd->max_groups; i++) {
                idxd->groups[i].idxd = idxd;
@@ -163,40 +221,31 @@ static int idxd_setup_internals(struct idxd_device *idxd)
                idxd->groups[i].tc_b = -1;
        }
 
-       idxd->wqs = devm_kcalloc(dev, idxd->max_wqs, sizeof(struct idxd_wq),
-                                GFP_KERNEL);
-       if (!idxd->wqs)
-               return -ENOMEM;
-
        idxd->engines = devm_kcalloc(dev, idxd->max_engines,
                                     sizeof(struct idxd_engine), GFP_KERNEL);
-       if (!idxd->engines)
-               return -ENOMEM;
-
-       for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
-
-               wq->id = i;
-               wq->idxd = idxd;
-               mutex_init(&wq->wq_lock);
-               wq->idxd_cdev.minor = -1;
-               wq->max_xfer_bytes = idxd->max_xfer_bytes;
-               wq->max_batch_size = idxd->max_batch_size;
-               wq->wqcfg = devm_kzalloc(dev, idxd->wqcfg_size, GFP_KERNEL);
-               if (!wq->wqcfg)
-                       return -ENOMEM;
+       if (!idxd->engines) {
+               rc = -ENOMEM;
+               goto err;
        }
 
+
        for (i = 0; i < idxd->max_engines; i++) {
                idxd->engines[i].idxd = idxd;
                idxd->engines[i].id = i;
        }
 
        idxd->wq = create_workqueue(dev_name(dev));
-       if (!idxd->wq)
-               return -ENOMEM;
+       if (!idxd->wq) {
+               rc = -ENOMEM;
+               goto err;
+       }
 
        return 0;
+
+ err:
+       for (i = 0; i < idxd->max_wqs; i++)
+               put_device(&idxd->wqs[i]->conf_dev);
+       return rc;
 }
 
 static void idxd_read_table_offsets(struct idxd_device *idxd)
@@ -371,11 +420,11 @@ static int idxd_probe(struct idxd_device *idxd)
 
        rc = idxd_setup_internals(idxd);
        if (rc)
-               goto err_setup;
+               goto err;
 
        rc = idxd_setup_interrupts(idxd);
        if (rc)
-               goto err_setup;
+               goto err;
 
        dev_dbg(dev, "IDXD interrupt setup complete.\n");
 
@@ -384,7 +433,7 @@ static int idxd_probe(struct idxd_device *idxd)
        dev_dbg(dev, "IDXD device %d probed successfully\n", idxd->id);
        return 0;
 
- err_setup:
+ err:
        if (device_pasid_enabled(idxd))
                idxd_disable_system_pasid(idxd);
        return rc;

diff --git a/drivers/dma/idxd/irq.c b/drivers/dma/idxd/irq.c
index f1463fc581125adf61e82e9b5042333779b585d8..7b0181532f770d38b8444b0f7ed938ebe679d95d 100644 (file)
--- a/drivers/dma/idxd/irq.c
+++ b/drivers/dma/idxd/irq.c
@@ -45,7 +45,7 @@ static void idxd_device_reinit(struct work_struct *work)
                goto out;
 
        for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
+               struct idxd_wq *wq = idxd->wqs[i];
 
                if (wq->state == IDXD_WQ_ENABLED) {
                        rc = idxd_wq_enable(wq);
@@ -130,7 +130,7 @@ static int process_misc_interrupts(struct idxd_device *idxd, u32 cause)
 
                if (idxd->sw_err.valid && idxd->sw_err.wq_idx_valid) {
                        int id = idxd->sw_err.wq_idx;
-                       struct idxd_wq *wq = &idxd->wqs[id];
+                       struct idxd_wq *wq = idxd->wqs[id];
 
                        if (wq->type == IDXD_WQT_USER)
                                wake_up_interruptible(&wq->idxd_cdev.err_queue);
@@ -138,7 +138,7 @@ static int process_misc_interrupts(struct idxd_device *idxd, u32 cause)
                        int i;
 
                        for (i = 0; i < idxd->max_wqs; i++) {
-                               struct idxd_wq *wq = &idxd->wqs[i];
+                               struct idxd_wq *wq = idxd->wqs[i];
 
                                if (wq->type == IDXD_WQT_USER)
                                        wake_up_interruptible(&wq->idxd_cdev.err_queue);

diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index 36193e555e364dfd7e51c16ed03a3ec68d6b8b87..409b3ce52f07ca879b69e8e1969bc12537ee14dd 100644 (file)
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -26,34 +26,11 @@ static struct device_type idxd_group_device_type = {
        .release = idxd_conf_sub_device_release,
 };
 
-static struct device_type idxd_wq_device_type = {
-       .name = "wq",
-       .release = idxd_conf_sub_device_release,
-};
-
 static struct device_type idxd_engine_device_type = {
        .name = "engine",
        .release = idxd_conf_sub_device_release,
 };
 
-static inline bool is_idxd_wq_dev(struct device *dev)
-{
-       return dev ? dev->type == &idxd_wq_device_type : false;
-}
-
-static inline bool is_idxd_wq_dmaengine(struct idxd_wq *wq)
-{
-       if (wq->type == IDXD_WQT_KERNEL &&
-           strcmp(wq->name, "dmaengine") == 0)
-               return true;
-       return false;
-}
-
-static inline bool is_idxd_wq_cdev(struct idxd_wq *wq)
-{
-       return wq->type == IDXD_WQT_USER;
-}
-
 static int idxd_config_bus_match(struct device *dev,
                                 struct device_driver *drv)
 {
@@ -297,7 +274,7 @@ static int idxd_config_bus_remove(struct device *dev)
                dev_dbg(dev, "%s removing dev %s\n", __func__,
                        dev_name(&idxd->conf_dev));
                for (i = 0; i < idxd->max_wqs; i++) {
-                       struct idxd_wq *wq = &idxd->wqs[i];
+                       struct idxd_wq *wq = idxd->wqs[i];
 
                        if (wq->state == IDXD_WQ_DISABLED)
                                continue;
@@ -309,7 +286,7 @@ static int idxd_config_bus_remove(struct device *dev)
                idxd_unregister_dma_device(idxd);
                rc = idxd_device_disable(idxd);
                for (i = 0; i < idxd->max_wqs; i++) {
-                       struct idxd_wq *wq = &idxd->wqs[i];
+                       struct idxd_wq *wq = idxd->wqs[i];
 
                        mutex_lock(&wq->wq_lock);
                        idxd_wq_disable_cleanup(wq);
@@ -678,7 +655,7 @@ static ssize_t group_work_queues_show(struct device *dev,
        struct idxd_device *idxd = group->idxd;
 
        for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
+               struct idxd_wq *wq = idxd->wqs[i];
 
                if (!wq->group)
                        continue;
@@ -935,7 +912,7 @@ static int total_claimed_wq_size(struct idxd_device *idxd)
        int wq_size = 0;
 
        for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
+               struct idxd_wq *wq = idxd->wqs[i];
 
                wq_size += wq->size;
        }
@@ -1331,6 +1308,20 @@ static const struct attribute_group *idxd_wq_attribute_groups[] = {
        NULL,
 };
 
+static void idxd_conf_wq_release(struct device *dev)
+{
+       struct idxd_wq *wq = container_of(dev, struct idxd_wq, conf_dev);
+
+       kfree(wq->wqcfg);
+       kfree(wq);
+}
+
+struct device_type idxd_wq_device_type = {
+       .name = "wq",
+       .release = idxd_conf_wq_release,
+       .groups = idxd_wq_attribute_groups,
+};
+
 /* IDXD device attribs */
 static ssize_t version_show(struct device *dev, struct device_attribute *attr,
                            char *buf)
@@ -1461,7 +1452,7 @@ static ssize_t clients_show(struct device *dev,
 
        spin_lock_irqsave(&idxd->dev_lock, flags);
        for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
+               struct idxd_wq *wq = idxd->wqs[i];
 
                count += wq->client_count;
        }
@@ -1711,70 +1702,67 @@ cleanup:
        return rc;
 }
 
-static int idxd_setup_wq_sysfs(struct idxd_device *idxd)
+static int idxd_register_wq_devices(struct idxd_device *idxd)
 {
-       struct device *dev = &idxd->pdev->dev;
-       int i, rc;
+       int i, rc, j;
 
        for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
-
-               wq->conf_dev.parent = &idxd->conf_dev;
-               dev_set_name(&wq->conf_dev, "wq%d.%d", idxd->id, wq->id);
-               wq->conf_dev.bus = idxd_get_bus_type(idxd);
-               wq->conf_dev.groups = idxd_wq_attribute_groups;
-               wq->conf_dev.type = &idxd_wq_device_type;
-               dev_dbg(dev, "WQ device register: %s\n",
-                       dev_name(&wq->conf_dev));
-               rc = device_register(&wq->conf_dev);
-               if (rc < 0) {
-                       put_device(&wq->conf_dev);
+               struct idxd_wq *wq = idxd->wqs[i];
+
+               rc = device_add(&wq->conf_dev);
+               if (rc < 0)
                        goto cleanup;
-               }
        }
 
        return 0;
 
 cleanup:
-       while (i--) {
-               struct idxd_wq *wq = &idxd->wqs[i];
+       j = i - 1;
+       for (; i < idxd->max_wqs; i++)
+               put_device(&idxd->wqs[i]->conf_dev);
 
-               device_unregister(&wq->conf_dev);
-       }
+       while (j--)
+               device_unregister(&idxd->wqs[j]->conf_dev);
        return rc;
 }
 
 int idxd_register_devices(struct idxd_device *idxd)
 {
        struct device *dev = &idxd->pdev->dev;
-       int rc;
+       int rc, i;
 
        rc = device_add(&idxd->conf_dev);
        if (rc < 0)
                return rc;
 
-       rc = idxd_setup_wq_sysfs(idxd);
+       rc = idxd_register_wq_devices(idxd);
        if (rc < 0) {
-               /* unregister conf dev */
-               dev_dbg(dev, "Work Queue sysfs registering failed: %d\n", rc);
-               return rc;
+               dev_dbg(dev, "WQ devices registering failed: %d\n", rc);
+               goto err_wq;
        }
 
        rc = idxd_setup_group_sysfs(idxd);
        if (rc < 0) {
                /* unregister conf dev */
                dev_dbg(dev, "Group sysfs registering failed: %d\n", rc);
-               return rc;
+               goto err;
        }
 
        rc = idxd_setup_engine_sysfs(idxd);
        if (rc < 0) {
                /* unregister conf dev */
                dev_dbg(dev, "Engine sysfs registering failed: %d\n", rc);
-               return rc;
+               goto err;
        }
 
        return 0;
+
+ err:
+       for (i = 0; i < idxd->max_wqs; i++)
+               device_unregister(&idxd->wqs[i]->conf_dev);
+ err_wq:
+       device_del(&idxd->conf_dev);
+       return rc;
 }
 
 void idxd_unregister_devices(struct idxd_device *idxd)
@@ -1782,7 +1770,7 @@ void idxd_unregister_devices(struct idxd_device *idxd)
        int i;
 
        for (i = 0; i < idxd->max_wqs; i++) {
-               struct idxd_wq *wq = &idxd->wqs[i];
+               struct idxd_wq *wq = idxd->wqs[i];
 
                device_unregister(&wq->conf_dev);
        }