Skip to content

Commit

Permalink
dma-buf: fix dma_buf_export init order v2
Browse files Browse the repository at this point in the history 
The init order and resulting error handling in dma_buf_export
was pretty messy.

Subordinate objects like the file and the sysfs kernel objects
were initializing and wiring itself up with the object in the
wrong order resulting not only in complicating and partially
incorrect error handling, but also in publishing only halve
initialized DMA-buf objects.

Clean this up thoughtfully by allocating the file independent
of the DMA-buf object. Then allocate and initialize the DMA-buf
object itself, before publishing it through sysfs. If everything
works as expected the file is then connected with the DMA-buf
object and publish it through debugfs.

Also adds the missing dma_resv_fini() into the error handling.

v2: add some missing changes to dma_bug_getfile() and a missing NULL
    check in dma_buf_file_release()

Signed-off-by: Christian König <[email protected]>
Reviewed-by: Michael J. Ruhl <[email protected]>
Reviewed-by: T.J. Mercier <[email protected]>
Acked-by: Sumit Semwal <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
  • Loading branch information
@ChristianKoenigAMD
ChristianKoenigAMD committed Dec 13, 2022
1 parent c425265 commit f728a5e
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 52 deletions.
7 changes: 2 additions & 5 deletions drivers/dma-buf/dma-buf-sysfs-stats.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,11 @@ void dma_buf_uninit_sysfs_statistics(void)
kset_unregister(dma_buf_stats_kset);
}

int dma_buf_stats_setup(struct dma_buf *dmabuf)
int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
{
struct dma_buf_sysfs_entry *sysfs_entry;
int ret;

if (!dmabuf || !dmabuf->file)
return -EINVAL;

if (!dmabuf->exp_name) {
pr_err("exporter name must not be empty if stats needed\n");
return -EINVAL;
Expand All @@ -192,7 +189,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)

/* create the directory for buffer stats */
ret = kobject_init_and_add(&sysfs_entry->kobj, &dma_buf_ktype, NULL,
"%lu", file_inode(dmabuf->file)->i_ino);
"%lu", file_inode(file)->i_ino);
if (ret)
goto err_sysfs_dmabuf;

Expand Down
4 changes: 2 additions & 2 deletions drivers/dma-buf/dma-buf-sysfs-stats.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
int dma_buf_init_sysfs_statistics(void);
void dma_buf_uninit_sysfs_statistics(void);

int dma_buf_stats_setup(struct dma_buf *dmabuf);
int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file);

void dma_buf_stats_teardown(struct dma_buf *dmabuf);
#else
Expand All @@ -25,7 +25,7 @@ static inline int dma_buf_init_sysfs_statistics(void)

static inline void dma_buf_uninit_sysfs_statistics(void) {}

static inline int dma_buf_stats_setup(struct dma_buf *dmabuf)
static inline int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
{
return 0;
}
Expand Down
84 changes: 39 additions & 45 deletions drivers/dma-buf/dma-buf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,11 @@ static int dma_buf_file_release(struct inode *inode, struct file *file)
return -EINVAL;

dmabuf = file->private_data;

mutex_lock(&db_list.lock);
list_del(&dmabuf->list_node);
mutex_unlock(&db_list.lock);
if (dmabuf) {
mutex_lock(&db_list.lock);
list_del(&dmabuf->list_node);
mutex_unlock(&db_list.lock);
}

return 0;
}
Expand Down Expand Up @@ -523,17 +524,17 @@ static inline int is_dma_buf_file(struct file *file)
return file->f_op == &dma_buf_fops;
}

static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
static struct file *dma_buf_getfile(size_t size, int flags)
{
static atomic64_t dmabuf_inode = ATOMIC64_INIT(0);
struct file *file;
struct inode *inode = alloc_anon_inode(dma_buf_mnt->mnt_sb);
struct file *file;

if (IS_ERR(inode))
return ERR_CAST(inode);

inode->i_size = dmabuf->size;
inode_set_bytes(inode, dmabuf->size);
inode->i_size = size;
inode_set_bytes(inode, size);

/*
* The ->i_ino acquired from get_next_ino() is not unique thus
Expand All @@ -547,8 +548,6 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
flags, &dma_buf_fops);
if (IS_ERR(file))
goto err_alloc_file;
file->private_data = dmabuf;
file->f_path.dentry->d_fsdata = dmabuf;

return file;

Expand Down Expand Up @@ -614,19 +613,11 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
size_t alloc_size = sizeof(struct dma_buf);
int ret;

if (!exp_info->resv)
alloc_size += sizeof(struct dma_resv);
else
/* prevent &dma_buf[1] == dma_buf->resv */
alloc_size += 1;

if (WARN_ON(!exp_info->priv
|| !exp_info->ops
|| !exp_info->ops->map_dma_buf
|| !exp_info->ops->unmap_dma_buf
|| !exp_info->ops->release)) {
if (WARN_ON(!exp_info->priv || !exp_info->ops
|| !exp_info->ops->map_dma_buf
|| !exp_info->ops->unmap_dma_buf
|| !exp_info->ops->release))
return ERR_PTR(-EINVAL);
}

if (WARN_ON(exp_info->ops->cache_sgt_mapping &&
(exp_info->ops->pin || exp_info->ops->unpin)))
Expand All @@ -638,10 +629,21 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
if (!try_module_get(exp_info->owner))
return ERR_PTR(-ENOENT);

file = dma_buf_getfile(exp_info->size, exp_info->flags);
if (IS_ERR(file)) {
ret = PTR_ERR(file);
goto err_module;
}

if (!exp_info->resv)
alloc_size += sizeof(struct dma_resv);
else
/* prevent &dma_buf[1] == dma_buf->resv */
alloc_size += 1;
dmabuf = kzalloc(alloc_size, GFP_KERNEL);
if (!dmabuf) {
ret = -ENOMEM;
goto err_module;
goto err_file;
}

dmabuf->priv = exp_info->priv;
Expand All @@ -653,44 +655,36 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
init_waitqueue_head(&dmabuf->poll);
dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll;
dmabuf->cb_in.active = dmabuf->cb_out.active = 0;
mutex_init(&dmabuf->lock);
INIT_LIST_HEAD(&dmabuf->attachments);

if (!resv) {
resv = (struct dma_resv *)&dmabuf[1];
dma_resv_init(resv);
dmabuf->resv = (struct dma_resv *)&dmabuf[1];
dma_resv_init(dmabuf->resv);
} else {
dmabuf->resv = resv;
}
dmabuf->resv = resv;

file = dma_buf_getfile(dmabuf, exp_info->flags);
if (IS_ERR(file)) {
ret = PTR_ERR(file);
ret = dma_buf_stats_setup(dmabuf, file);
if (ret)
goto err_dmabuf;
}

file->private_data = dmabuf;
file->f_path.dentry->d_fsdata = dmabuf;
dmabuf->file = file;

mutex_init(&dmabuf->lock);
INIT_LIST_HEAD(&dmabuf->attachments);

mutex_lock(&db_list.lock);
list_add(&dmabuf->list_node, &db_list.head);
mutex_unlock(&db_list.lock);

ret = dma_buf_stats_setup(dmabuf);
if (ret)
goto err_sysfs;

return dmabuf;

err_sysfs:
/*
* Set file->f_path.dentry->d_fsdata to NULL so that when
* dma_buf_release() gets invoked by dentry_ops, it exits
* early before calling the release() dma_buf op.
*/
file->f_path.dentry->d_fsdata = NULL;
fput(file);
err_dmabuf:
if (!resv)
dma_resv_fini(dmabuf->resv);
kfree(dmabuf);
err_file:
fput(file);
err_module:
module_put(exp_info->owner);
return ERR_PTR(ret);
Expand Down

0 comments on commit f728a5e

Please sign in to comment.