Re: [PATCH v4 14/14] io_uring/rsrc: add dmabuf backed registered buffers
Pavel Begunkov <[email protected]> Wed, 29 Jul 2026 14:43:34 +0100
| Newsgroups | dev.linux.lists.dm-devel,dev.linux.lists.nvdimm,org.freedesktop.lists.dri-devel,org.infradead.lists.linux-nvme,org.kernel.vger.ceph-devel,org.kernel.vger.io-uring,org.kernel.vger.linux-block,org.kernel.vger.linux-btrfs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media |
|---|---|
| Message-ID | <[email protected]> |
On 7/29/26 14:30, Anuj Gupta/Anuj Gupta wrote:
> On 7/29/2026 2:59 AM, Pavel Begunkov wrote:
>> +static struct io_rsrc_node *io_register_dmabuf(struct io_ring_ctx *ctx,
>> + struct io_uring_regbuf_desc *desc)
>> +{
>> + struct io_rsrc_node *node = NULL;
>> + struct io_mapped_ubuf *imu = NULL;
>> + struct io_regbuf_dma *regbuf = NULL;
>> + struct file *target_file = NULL;
>> + struct dma_buf *dmabuf = NULL;
>> + int ret;
>> +
>> + if (!IS_ENABLED(CONFIG_DMA_SHARED_BUFFER))
>> + return ERR_PTR(-EOPNOTSUPP);
>> + if (ctx->flags & IORING_SETUP_IOPOLL)
>> + return ERR_PTR(-EOPNOTSUPP);
>> + if (desc->uaddr || desc->size)
>> + return ERR_PTR(-EINVAL);
>> +
>> + ret = -ENOMEM;
>> + node = io_rsrc_node_alloc(ctx, IORING_RSRC_BUFFER);
>> + if (!node)
>> + return ERR_PTR(-ENOMEM);
>> + imu = io_alloc_imu(ctx, 0);
>> + if (!imu)
>> + goto err;
>> + regbuf = kzalloc(sizeof(*regbuf), GFP_KERNEL);
>> + if (!regbuf)
>> + goto err;
>> +
>> + ret = -EBADF;
>> + target_file = fget(desc->target_fd);
>> + if (!target_file)
>> + goto err;
>> +
>> + dmabuf = dma_buf_get(desc->dmabuf_fd);
>> + if (IS_ERR(dmabuf)) {
>> + ret = PTR_ERR(dmabuf);
>> + dmabuf = NULL;
>> + goto err;
>> + }
>> + if (dmabuf->size > SZ_1G) {
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + ret = dma_buf_io_ctx_create(target_file, ®buf->ctx, dmabuf,
>> + DMA_BIDIRECTIONAL);
>> + if (ret)
>> + goto err;
>> +
>> + regbuf->target_file = target_file;
>> + imu->nr_bvecs = 1;
>
> There is no bvec backing a dmabuf imu - should this be 0?
Doesn't really matter as it's not used. It's 1 to say that
it's just 1 contig segment, but I can just zero it to avoid
confusion.
>
>> + imu->ubuf = 0;
>> + imu->len = dmabuf->size;
>> + imu->folio_shift = 0;
>> + imu->release = io_release_reg_dmabuf;
>> + imu->priv = regbuf;
>> + imu->flags = IO_REGBUF_F_DMABUF;
>> + imu->dir = IO_IMU_DEST | IO_IMU_SOURCE;
>> + refcount_set(&imu->refs, 1);
>> + node->buf = imu;
>> + dma_buf_put(dmabuf);
>> + return node;
>> +err:
>> + kfree(regbuf);
>> + if (imu)
>> + io_free_imu(ctx, imu);
>> + if (node)
>> + io_cache_free(&ctx->node_cache, node);
>> + if (target_file)
>> + fput(target_file);
>> + if (dmabuf)
>> + dma_buf_put(dmabuf);
>> + return ERR_PTR(ret);
>> +}
>> +
>> +
>
> nit: extra newline here
>
>> +static int io_import_dmabuf(struct io_kiocb *req,
>> + int ddir, struct iov_iter *iter,
>> + struct io_mapped_ubuf *imu,
>> + size_t len, size_t offset,
>> + unsigned issue_flags)
>> +{
>> + struct io_regbuf_dma *db = imu->priv;
>> + struct dma_buf_io_map *map;
>> +
>> + if (!IS_ENABLED(CONFIG_DMA_SHARED_BUFFER))
>> + return -EOPNOTSUPP;
>> + if (!len)
>> + return -EFAULT;
>> + if (req->file != db->target_file)
>> + return -EBADF;
>> +
>> + if (req->flags & REQ_F_DROP_DMABUF) {
>> + map = req->dmabuf_map;
>> + goto init_iter;
>> + }
>
> O_DIRECT is checked only during registration; we should recheck
> (file->f_flags & O_DIRECT) per request.
Nobody should be able to clear O_DIRECT, unless I missed something?
And fwiw, io_uring wouldn't be the right place to do it.
--
Pavel Begunkov