dmaengine: idxd: fix cdev setup and free device lifetime issues

[ Upstream commit 04922b7445a1950b86f130a1fe8c52cc27b3e30b ]

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@nvidia.com>
Fixes: 42d279f913 ("dmaengine: idxd: add char driver to expose submission portal to userland")
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Link: https://lore.kernel.org/r/161852987721.2203940.1478218825576630810.stgit@djiang5-desk3.ch.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Dave Jiang 2021-04-15 16:37:57 -07:00 committed by Greg Kroah-Hartman
parent 975c4b2b99
commit dd41a0e515
5 changed files with 63 additions and 89 deletions

View File

@ -35,15 +35,15 @@ struct idxd_user_context {
unsigned int flags;
};
enum idxd_cdev_cleanup {
CDEV_NORMAL = 0,
CDEV_FAILED,
};
static void idxd_cdev_dev_release(struct device *dev)
{
dev_dbg(dev, "releasing cdev device\n");
kfree(dev);
struct idxd_cdev *idxd_cdev = container_of(dev, struct idxd_cdev, dev);
struct idxd_cdev_context *cdev_ctx;
struct idxd_wq *wq = idxd_cdev->wq;
cdev_ctx = &ictx[wq->idxd->type];
ida_simple_remove(&cdev_ctx->minor_ida, idxd_cdev->minor);
kfree(idxd_cdev);
}
static struct device_type idxd_cdev_device_type = {
@ -58,14 +58,11 @@ static inline struct idxd_cdev *inode_idxd_cdev(struct inode *inode)
return container_of(cdev, struct idxd_cdev, cdev);
}
static inline struct idxd_wq *idxd_cdev_wq(struct idxd_cdev *idxd_cdev)
{
return container_of(idxd_cdev, struct idxd_wq, idxd_cdev);
}
static inline struct idxd_wq *inode_wq(struct inode *inode)
{
return idxd_cdev_wq(inode_idxd_cdev(inode));
struct idxd_cdev *idxd_cdev = inode_idxd_cdev(inode);
return idxd_cdev->wq;
}
static int idxd_cdev_open(struct inode *inode, struct file *filp)
@ -172,11 +169,10 @@ static __poll_t idxd_cdev_poll(struct file *filp,
struct idxd_user_context *ctx = filp->private_data;
struct idxd_wq *wq = ctx->wq;
struct idxd_device *idxd = wq->idxd;
struct idxd_cdev *idxd_cdev = &wq->idxd_cdev;
unsigned long flags;
__poll_t out = 0;
poll_wait(filp, &idxd_cdev->err_queue, wait);
poll_wait(filp, &wq->err_queue, wait);
spin_lock_irqsave(&idxd->dev_lock, flags);
if (idxd->sw_err.valid)
out = EPOLLIN | EPOLLRDNORM;
@ -198,98 +194,67 @@ int idxd_cdev_get_major(struct idxd_device *idxd)
return MAJOR(ictx[idxd->type].devt);
}
static int idxd_wq_cdev_dev_setup(struct idxd_wq *wq)
int idxd_wq_add_cdev(struct idxd_wq *wq)
{
struct idxd_device *idxd = wq->idxd;
struct idxd_cdev *idxd_cdev = &wq->idxd_cdev;
struct idxd_cdev_context *cdev_ctx;
struct idxd_cdev *idxd_cdev;
struct cdev *cdev;
struct device *dev;
int minor, rc;
struct idxd_cdev_context *cdev_ctx;
int rc, minor;
idxd_cdev->dev = kzalloc(sizeof(*idxd_cdev->dev), GFP_KERNEL);
if (!idxd_cdev->dev)
idxd_cdev = kzalloc(sizeof(*idxd_cdev), GFP_KERNEL);
if (!idxd_cdev)
return -ENOMEM;
dev = idxd_cdev->dev;
dev->parent = &idxd->pdev->dev;
dev_set_name(dev, "%s/wq%u.%u", idxd_get_dev_name(idxd),
idxd->id, wq->id);
dev->bus = idxd_get_bus_type(idxd);
idxd_cdev->wq = wq;
cdev = &idxd_cdev->cdev;
dev = &idxd_cdev->dev;
cdev_ctx = &ictx[wq->idxd->type];
minor = ida_simple_get(&cdev_ctx->minor_ida, 0, MINORMASK, GFP_KERNEL);
if (minor < 0) {
rc = minor;
kfree(dev);
goto ida_err;
}
dev->devt = MKDEV(MAJOR(cdev_ctx->devt), minor);
dev->type = &idxd_cdev_device_type;
rc = device_register(dev);
if (rc < 0) {
dev_err(&idxd->pdev->dev, "device register failed\n");
goto dev_reg_err;
kfree(idxd_cdev);
return minor;
}
idxd_cdev->minor = minor;
return 0;
device_initialize(dev);
dev->parent = &wq->conf_dev;
dev->bus = idxd_get_bus_type(idxd);
dev->type = &idxd_cdev_device_type;
dev->devt = MKDEV(MAJOR(cdev_ctx->devt), minor);
dev_reg_err:
ida_simple_remove(&cdev_ctx->minor_ida, MINOR(dev->devt));
put_device(dev);
ida_err:
idxd_cdev->dev = NULL;
return rc;
}
static void idxd_wq_cdev_cleanup(struct idxd_wq *wq,
enum idxd_cdev_cleanup cdev_state)
{
struct idxd_cdev *idxd_cdev = &wq->idxd_cdev;
struct idxd_cdev_context *cdev_ctx;
cdev_ctx = &ictx[wq->idxd->type];
if (cdev_state == CDEV_NORMAL)
cdev_del(&idxd_cdev->cdev);
device_unregister(idxd_cdev->dev);
/*
* The device_type->release() will be called on the device and free
* the allocated struct device. We can just forget it.
*/
ida_simple_remove(&cdev_ctx->minor_ida, idxd_cdev->minor);
idxd_cdev->dev = NULL;
idxd_cdev->minor = -1;
}
int idxd_wq_add_cdev(struct idxd_wq *wq)
{
struct idxd_cdev *idxd_cdev = &wq->idxd_cdev;
struct cdev *cdev = &idxd_cdev->cdev;
struct device *dev;
int rc;
rc = idxd_wq_cdev_dev_setup(wq);
rc = dev_set_name(dev, "%s/wq%u.%u", idxd_get_dev_name(idxd),
idxd->id, wq->id);
if (rc < 0)
return rc;
goto err;
dev = idxd_cdev->dev;
wq->idxd_cdev = idxd_cdev;
cdev_init(cdev, &idxd_cdev_fops);
cdev_set_parent(cdev, &dev->kobj);
rc = cdev_add(cdev, dev->devt, 1);
rc = cdev_device_add(cdev, dev);
if (rc) {
dev_dbg(&wq->idxd->pdev->dev, "cdev_add failed: %d\n", rc);
idxd_wq_cdev_cleanup(wq, CDEV_FAILED);
return rc;
goto err;
}
init_waitqueue_head(&idxd_cdev->err_queue);
return 0;
err:
put_device(dev);
wq->idxd_cdev = NULL;
return rc;
}
void idxd_wq_del_cdev(struct idxd_wq *wq)
{
idxd_wq_cdev_cleanup(wq, CDEV_NORMAL);
struct idxd_cdev *idxd_cdev;
struct idxd_cdev_context *cdev_ctx;
cdev_ctx = &ictx[wq->idxd->type];
idxd_cdev = wq->idxd_cdev;
wq->idxd_cdev = NULL;
cdev_device_del(&idxd_cdev->cdev, &idxd_cdev->dev);
put_device(&idxd_cdev->dev);
}
int idxd_cdev_register(void)

View File

@ -71,10 +71,10 @@ enum idxd_wq_type {
};
struct idxd_cdev {
struct idxd_wq *wq;
struct cdev cdev;
struct device *dev;
struct device dev;
int minor;
struct wait_queue_head err_queue;
};
#define IDXD_ALLOCATED_BATCH_SIZE 128U
@ -99,7 +99,8 @@ struct idxd_dma_chan {
struct idxd_wq {
void __iomem *dportal;
struct device conf_dev;
struct idxd_cdev idxd_cdev;
struct idxd_cdev *idxd_cdev;
struct wait_queue_head err_queue;
struct idxd_device *idxd;
int id;
enum idxd_wq_type type;

View File

@ -175,7 +175,7 @@ static int idxd_setup_internals(struct idxd_device *idxd)
wq->id = i;
wq->idxd = idxd;
mutex_init(&wq->wq_lock);
wq->idxd_cdev.minor = -1;
init_waitqueue_head(&wq->err_queue);
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);

View File

@ -75,7 +75,7 @@ static int process_misc_interrupts(struct idxd_device *idxd, u32 cause)
struct idxd_wq *wq = &idxd->wqs[id];
if (wq->type == IDXD_WQT_USER)
wake_up_interruptible(&wq->idxd_cdev.err_queue);
wake_up_interruptible(&wq->err_queue);
} else {
int i;
@ -83,7 +83,7 @@ static int process_misc_interrupts(struct idxd_device *idxd, u32 cause)
struct idxd_wq *wq = &idxd->wqs[i];
if (wq->type == IDXD_WQT_USER)
wake_up_interruptible(&wq->idxd_cdev.err_queue);
wake_up_interruptible(&wq->err_queue);
}
}

View File

@ -1052,8 +1052,16 @@ 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);
if (minor == -1)
return -ENXIO;
return sysfs_emit(buf, "%d\n", minor);
}
static struct device_attribute dev_attr_wq_cdev_minor =