Re: [PATCH v7 8/8] dmaengine: idxd: fix cdev setup and free device lifetime issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3/22/2021 5:02 PM, Jason Gunthorpe wrote:
On Mon, Mar 22, 2021 at 04:44:24PM -0700, Dave Jiang wrote:
On 3/22/2021 4:38 PM, Dan Williams wrote:
On Mon, Mar 22, 2021 at 4:32 PM Dave Jiang <dave.jiang@xxxxxxxxx> wrote:
The char device setup and cleanup has device lifetime issues regarding when
parts are initialized and cleaned up. The initialization of struct device is
done incorrectly. device_initialize() needs to be called on the 'struct
device' and then additional changes can be added. The ->release() function
needs to be setup via device_type before dev_set_name() to allow proper
cleanup. The change re-parents the cdev under the wq->conf_dev to get
natural reference inheritance. No known dependency on the old device path exists.

Reported-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Fixes: 42d279f9137a ("dmaengine: idxd: add char driver to expose submission portal to userland")
Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
[..]
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index 03079ff54889..8a08988ea9d1 100644
+++ b/drivers/dma/idxd/sysfs.c
@@ -1174,8 +1174,14 @@ static ssize_t wq_cdev_minor_show(struct device *dev,
                                    struct device_attribute *attr, char *buf)
   {
          struct idxd_wq *wq = container_of(dev, struct idxd_wq, conf_dev);
+       int minor = -1;

-       return sprintf(buf, "%d\n", wq->idxd_cdev.minor);
+       mutex_lock(&wq->wq_lock);
+       if (wq->idxd_cdev)
+               minor = wq->idxd_cdev->minor;
+       mutex_unlock(&wq->wq_lock);
+
+       return sprintf(buf, "%d\n", minor);
As I mentioned, let's not emit a negative value here. ...not that
userspace should be using this awkward recreation of the existing core
'dev' attribute anyway.

if (minor == -1)
      return -ENXIO;
return sprintf(buf, "%d\n", minor);
Ok I'll update. This will go away when we convert to UACCE based driver.
Also ensure all new syfs callbacks are using sysfs_emit()

Thanks Jason! I have posted a patch that fixes all the instances [1]. But I can fix it locally here as well.


[1]: https://lore.kernel.org/dmaengine/161495936863.573579.5804720281778882171.stgit@xxxxxxxxxxxxxxxxxxxxxxxxxx/





[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux