io_uring dmabuf read/write support
Subsystems (7)
cover io_uring dmabuf read/write support
Disclaimer: haven't been tested well enough yet and needs more beating
For past couple of months David Wei, Vishal Verma and other folks have been mentioning that it'd be great to have dmabuf support for read/write and other operations in io_uring. The topic is not new, it has been discussed many times in different contexts including networking. The last relevant attempt was premapped dma tags by Keith [1], and this patch set took a lot from it.
This series implements it for read/write io_uring requests. The uAPI looks similar to normal registered buffers, the user will need to register a dmabuf in io_uring first and then use it as any other registered buffer. On registration the user also specifies a file to map the dmabuf for.
// registerio_uring_update_buffers(ring, { dma_buf_fd, target_file_fd }); // usereg_buf_idx = 0;
io_uring_prep_read_fixed(sqe, target_file_fd, buffer_offset,
buffer_size, file_offset, reg_buf_idx);
It's an RFC to discuss the overall direction. The series misses parts like bio splitting and nvme sgl support, and otherwise there are some rough edges and probably problems, which will need more testing and attention.
[1] https://lore.kernel.org/io-uring/[email protected]/
simple liburing based example:
git: https://github.com/isilence/liburing.git dmabuf-rw
kernel branch:
git: https://github.com/isilence/linux.git dmabuf-rw-v1
Pavel Begunkov (12):
file: add callback returning dev for dma operations
iov_iter: introduce iter type for pre-registered dma
block: move around bio flagging helpers
block: introduce dmavec bio type
block: implement ->get_dma_device callback
nvme-pci: add support for user passed dma vectors io_uring/rsrc: extended reg buffer registration
io_uring: add basic dmabuf helpers io_uring/rsrc: add imu flags
io_uring/rsrc: add dmabuf-backed buffer registeration io_uring/rsrc: implement dmabuf regbuf import io_uring/rw: enable dma registered buffers
block/bdev.c | 11 ++ block/bio.c | 21 ++++ block/blk-merge.c | 32 +++++ block/blk.h | 2 +- block/fops.c | 3 + drivers/nvme/host/pci.c | 158 +++++++++++++++++++++++ include/linux/bio.h | 59 ++++++--- include/linux/blk-mq.h | 2 + include/linux/blk_types.h | 6 +- include/linux/blkdev.h | 2 + include/linux/fs.h | 2 + include/linux/uio.h | 14 +++ include/uapi/linux/io_uring.h | 13 +- io_uring/Makefile | 1 + io_uring/dmabuf.c | 60 +++++++++ io_uring/dmabuf.h | 34 +++++ io_uring/rsrc.c | 230 ++++++++++++++++++++++++++++++---- io_uring/rsrc.h | 23 +++- io_uring/rw.c | 7 +- lib/iov_iter.c | 70 ++++++++++- 20 files changed, 701 insertions(+), 49 deletions(-) create mode 100644 io_uring/dmabuf.c create mode 100644 io_uring/dmabuf.h
Signature · 1 line
2.49.0
RFC 1/12 file: add callback returning dev for dma operations
Add a file callback returning a device that can be used for dma pre-mapping buffers. There should be only one device per file, and a file should never return different deivecs during its lifetime.
--- include/linux/fs.h | 2 ++ 1 file changed, 2 insertions(+)
include/linux/fs.h +2 -0
@@ -82,6 +82,7 @@ struct fs_context;struct fs_parameter_spec ;struct fileattr ;struct iomap_ops ; +struct device ;extern void __init inode_init (void );extern void __init inode_init_early (void );
@@ -2190,6 +2191,7 @@ struct file_operations {int (*uring_cmd_iopoll)(struct io_uring_cmd *,struct io_comp_batch *,unsigned int poll_flags);int (*mmap_prepare)(struct vm_area_desc *); +struct device *(*get_dma_device)(struct file *); } __randomize_layout;/* Supports async buffered reads */
Signature · 1 line
2.49.0
RFC 2/12 iov_iter: introduce iter type for pre-registered dma
In reply to Pavel BegunkovIntroduce a new iterator type representing vectors with pre-registered DMA addresses. It carries an array of struct dmavec, which is just a {dma addr, dma len} pair. It'll be used to pass dmabuf buffers from io_uring and other interfaces operating with iterators.
The vector is mapped for the device returned by the ->get_dma_device() callback of the file, and the caller should only pass the iterator to that file's methods. That should also prevent ITER_DMAVEC iterators reaching unaware files.
Note, the drivers are responsible for cpu-device memory synchronisation and should use dma_sync_single_for_{device,cpu} when appropriate.
--- include/linux/uio.h | 14 +++++++++ lib/iov_iter.c | 70 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-)
include/linux/uio.h +14 -0
@@ -29,11 +29,17 @@ enum iter_type { ITER_FOLIOQ,ITER_XARRAY ,ITER_DISCARD , + ITER_DMAVEC, };#define ITER_SOURCE 1 // == WRITE#define ITER_DEST 0 // == READ +struct dmavec { +dma_addr_t addr; +int len; +}; +struct iov_iter_state {size_t iov_offset;size_t count;
@@ -71,6 +77,7 @@ struct iov_iter {const struct folio_queue *folioq;struct xarray *xarray;void __user *ubuf; +const struct dmavec *dmavec; };size_t count; };
@@ -155,6 +162,11 @@ static inline bool iov_iter_is_xarray(const struct iov_iter *i)return iov_iter_type (i) ==ITER_XARRAY ; } +static inline bool iov_iter_is_dma (const struct iov_iter *i) +{ +return iov_iter_type (i) ==ITER_DMAVEC ; +} +static inline unsigned char iov_iter_rw (const struct iov_iter *i) {return i->data_source ?WRITE :READ ;
@@ -302,6 +314,8 @@ void iov_iter_folio_queue(struct iov_iter *i, unsigned int direction, unsigned int first_slot, unsigned int offset, size_t count);void iov_iter_xarray (struct iov_iter *i,unsigned int direction,struct xarray *xarray,loff_t start,size_t count); +void iov_iter_dma (struct iov_iter *i,unsigned int direction, +struct dmavec *dmavec,unsigned nr_segs,size_t count);ssize_t iov_iter_get_pages2 (struct iov_iter *i,struct page **pages,size_t maxsize,unsigned maxpages,size_t *start); ssize_t iov_iter_get_pages_alloc2(struct iov_iter *i, struct page ***pages,
lib/iov_iter.c +69 -1
112 lines not on this page · raw message
Signature · 1 line
2.49.0
RFC 3/12 block: move around bio flagging helpers
In reply to Pavel BegunkovWe'll need bio_flagged() earlier in bio.h in the next patch, move it together with all related helpers, and mark the bio_flagged()'s bio argument as const.
--- include/linux/bio.h | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
include/linux/bio.h +15 -15
@@ -46,6 +46,21 @@ static inline unsigned int bio_max_segs(unsigned int nr_segs)#define bio_data_dir (bio) \ (op_is_write(bio_op(bio)) ? WRITE : READ) +static inline bool bio_flagged (const struct bio *bio,unsigned int bit) +{ +return bio->bi_flags & (1U << bit); +} + +static inline void bio_set_flag (struct bio *bio,unsigned int bit) +{ + bio->bi_flags |= (1U << bit); +} + +static inline void bio_clear_flag (struct bio *bio,unsigned int bit) +{ + bio->bi_flags &= ~(1U << bit); +} +/* * Check whether this bio carries any data or not. A NULL bio is allowed. */
@@ -225,21 +240,6 @@ static inline void bio_cnt_set(struct bio *bio, unsigned int count)atomic_set (&bio->__bi_cnt, count); } -static inline bool bio_flagged (struct bio *bio,unsigned int bit) -{ -return bio->bi_flags & (1U << bit); -} - -static inline void bio_set_flag (struct bio *bio,unsigned int bit) -{ - bio->bi_flags |= (1U << bit); -} - -static inline void bio_clear_flag (struct bio *bio,unsigned int bit) -{ - bio->bi_flags &= ~(1U << bit); -} -static inline struct bio_vec *bio_first_bvec_all (struct bio *bio) {WARN_ON_ONCE (bio_flagged (bio,BIO_CLONED ));
Signature · 1 line
2.49.0
RFC 4/12 block: introduce dmavec bio type
In reply to Pavel BegunkovPremapped buffers don't require a generic bio_vec since these have already been dma mapped. Repurpose the bi_io_vec with the dma vector as they are mutually exclusive, and provide the setup and to support dma vectors.
In order to use this, a driver must implement the .get_dma_device() blk-mq op. If the driver provides this callback, then it must be aware that any given bio may be using a dma_tag instead of a bio_vec.
Note, splitting is not implemented just yet.
--- block/bio.c | 21 +++++++++++++++++++++ block/blk-merge.c | 32 ++++++++++++++++++++++++++++++++ block/blk.h | 2 +- block/fops.c | 2 ++ include/linux/bio.h | 29 ++++++++++++++++++++++++++--- include/linux/blk_types.h | 6 +++++- 6 files changed, 87 insertions(+), 5 deletions(-)
block/bio.c +21 -0
@@ -838,6 +838,9 @@ static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp)bio_clone_blkg_association (bio, bio_src); } +if (bio_flagged (bio_src,BIO_DMAVEC )) +bio_set_flag (bio,BIO_DMAVEC ); +if (bio_crypt_clone (bio, bio_src, gfp) <0 )return -ENOMEM ; if (bio_integrity(bio_src) &&
@@ -1156,6 +1159,18 @@ void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter)bio_set_flag (bio,BIO_CLONED ); } +void bio_iov_dmavec_set (struct bio *bio,struct iov_iter *iter) +{ +WARN_ON_ONCE (bio->bi_max_vecs); + + bio->bi_vcnt = iter->nr_segs; + bio->bi_dmavec = (struct dmavec *)iter->dmavec; + bio->bi_iter.bi_bvec_done = iter->iov_offset; + bio->bi_iter.bi_size =iov_iter_count (iter); + bio->bi_opf |=REQ_NOMERGE ; +bio_set_flag (bio,BIO_DMAVEC ); +} + static unsigned int get_contig_folio_len(unsigned int *num_pages, struct page **pages, unsigned int i, struct folio *folio, size_t left,
@@ -1322,6 +1337,10 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)bio_iov_bvec_set (bio, iter);iov_iter_advance (iter, bio->bi_iter.bi_size);return 0 ; + }else if (iov_iter_is_dma (iter)) { +bio_iov_dmavec_set (bio, iter); +iov_iter_advance (iter, bio->bi_iter.bi_size); +return 0 ; }if (iov_iter_extract_will_pin (iter))
@@ -1673,6 +1692,8 @@ struct bio *bio_split(struct bio *bio, int sectors,/* Zone append commands cannot be split */ if (WARN_ON_ONCE (bio_op (bio) ==REQ_OP_ZONE_APPEND ))return ERR_PTR (-EINVAL ); +if (WARN_ON_ONCE (bio_flagged (bio,BIO_DMAVEC ))) +return ERR_PTR (-EINVAL );/* atomic writes cannot be split */ if (bio->bi_opf &REQ_ATOMIC )
block/blk-merge.c +32 -0
@@ -278,6 +278,35 @@ static unsigned int bio_split_alignment(struct bio *bio,return lim->logical_block_size; } +static int bio_split_rw_at_dmavec (struct bio *bio,const struct queue_limits *lim, +unsigned *segs,unsigned max_bytes) +{ +struct dmavec *dmav; +int offset, length; + +/* Aggressively refuse any splitting, should be improved */ + +if (!lim->virt_boundary_mask) +return -EINVAL ; +if (bio->bi_vcnt > lim->max_segments) +return -EINVAL ; +if (bio->bi_iter.bi_size > max_bytes) +return -EINVAL ; + + dmav = &bio->bi_dmavec[bio->bi_iter.bi_idx]; + offset = bio->bi_iter.bi_bvec_done; + length = bio->bi_iter.bi_size; +while (length >0 ) { +if (dmav->len - offset > lim->max_segment_size) +return -EINVAL ; + length -= dmav->len; + dmav++; + } + *segs = bio->bi_vcnt; +return 0 ; +} + +/** * bio_split_rw_at - check if and where to split a read/write bio * @bio: [in] bio to be split
@@ -297,6 +326,9 @@ int bio_split_rw_at(struct bio *bio, const struct queue_limits *lim,struct bvec_iter iter;unsigned nsegs =0 , bytes =0 ; +if (bio_flagged (bio,BIO_DMAVEC )) +return bio_split_rw_at_dmavec (bio, lim, segs, max_bytes); +bio_for_each_bvec (bv, bio, iter) {/* * If the queue doesn't support SG gaps and adding this
block/blk.h +1 -1
@@ -386,7 +386,7 @@ static inline struct bio *__bio_split_to_limits(struct bio *bio,switch (bio_op (bio)) {case REQ_OP_READ :case REQ_OP_WRITE : -if (bio_may_need_split (bio, lim)) +if (bio_may_need_split (bio, lim) ||bio_flagged (bio,BIO_DMAVEC ))return bio_split_rw (bio, lim, nr_segs); *nr_segs =1 ;return bio;
block/fops.c +2 -0
@@ -347,6 +347,8 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,* bio_iov_iter_get_pages() and set the bvec directly. */ bio_iov_bvec_set (bio, iter); + }else if (iov_iter_is_dma (iter)) { +bio_iov_dmavec_set (bio, iter); }else { ret =bio_iov_iter_get_pages (bio, iter);if (unlikely (ret))
include/linux/bio.h +26 -3
@@ -108,16 +108,36 @@ static inline bool bio_next_segment(const struct bio *bio,#define bio_for_each_segment_all (bvl, bio, iter) \ for (bvl = bvec_init_iter_all(&iter); bio_next_segment((bio), &iter); ) +static inline void bio_advance_iter_dma (const struct bio *bio, +struct bvec_iter *iter,unsigned int bytes) +{ +unsigned int idx = iter->bi_idx; + + iter->bi_size -= bytes; + bytes += iter->bi_bvec_done; + +while (bytes && bytes >= bio->bi_dmavec[idx].len) { + bytes -= bio->bi_dmavec[idx].len; + idx++; + } + + iter->bi_idx = idx; + iter->bi_bvec_done = bytes; +} +static inline void bio_advance_iter (const struct bio *bio,struct bvec_iter *iter,unsigned int bytes) { iter->bi_sector += bytes >>9 ; -if (bio_no_advance_iter (bio)) +if (bio_no_advance_iter (bio)) { iter->bi_size -= bytes; -else + }else if (bio_flagged (bio,BIO_DMAVEC )) { +bio_advance_iter_dma (bio, iter, bytes); + }else {bvec_iter_advance (bio->bi_io_vec, iter, bytes);/* TODO: It is reasonable to complete bio with error here. */ + } }/* @bytes should be less or equal to bvec[i->bi_idx].bv_len */
@@ -129,6 +149,8 @@ static inline void bio_advance_iter_single(const struct bio *bio,if (bio_no_advance_iter (bio)) iter->bi_size -= bytes; +else if (bio_flagged (bio,BIO_DMAVEC )) +bio_advance_iter_dma (bio, iter, bytes);else bvec_iter_advance_single (bio->bi_io_vec, iter, bytes); }
@@ -396,7 +418,7 @@ static inline void bio_wouldblock_error(struct bio *bio)*/ static inline int bio_iov_vecs_to_alloc (struct iov_iter *iter,int max_segs) { -if (iov_iter_is_bvec (iter)) +if (iov_iter_is_bvec (iter) ||iov_iter_is_dma (iter))return 0 ;return iov_iter_npages (iter, max_segs); }
@@ -443,6 +465,7 @@ int bdev_rw_virt(struct block_device *bdev, sector_t sector, void *data,int bio_iov_iter_get_pages (struct bio *bio,struct iov_iter *iter);void bio_iov_bvec_set (struct bio *bio,const struct iov_iter *iter); +void bio_iov_dmavec_set (struct bio *bio,struct iov_iter *iter);void __bio_release_pages (struct bio *bio,bool mark_dirty);extern void bio_set_pages_dirty (struct bio *bio);extern void bio_check_pages_dirty (struct bio *bio);
include/linux/blk_types.h +5 -1
@@ -266,7 +266,10 @@ struct bio {atomic_t __bi_cnt;/* pin count */ -struct bio_vec *bi_io_vec;/* the actual vec list */ +union { +struct bio_vec *bi_io_vec;/* the actual vec list */ +struct dmavec *bi_dmavec; + };struct bio_set *bi_pool;
@@ -308,6 +311,7 @@ enum {BIO_REMAPPED ,BIO_ZONE_WRITE_PLUGGING ,/* bio handled through zone write plugging */ BIO_EMULATES_ZONE_APPEND ,/* bio emulates a zone append operation */ +BIO_DMAVEC ,/* Using premmaped dma buffers */ BIO_FLAG_LAST };
Signature · 1 line
2.49.0
RFC 5/12 block: implement ->get_dma_device callback
In reply to Pavel BegunkovImplement the ->get_dma_device callback for block files by forwarding it to drivers via a new blk-mq ops callback.
--- block/bdev.c | 11 +++++++++++ block/fops.c | 1 + include/linux/blk-mq.h | 2 ++ include/linux/blkdev.h | 2 ++ 4 files changed, 16 insertions(+)
block/bdev.c +11 -0
@@ -61,6 +61,17 @@ struct block_device *file_bdev(struct file *bdev_file) }EXPORT_SYMBOL (file_bdev); +struct device *block_get_dma_device (struct file *file) +{ +struct request_queue *q =bdev_get_queue (file_bdev (file)); + +if (!(file->f_flags &O_DIRECT )) +return ERR_PTR (-EINVAL ); +if (q->mq_ops && q->mq_ops->get_dma_device) +return q->mq_ops->get_dma_device (q); +return ERR_PTR (-EINVAL ); +} +static void bdev_write_inode (struct block_device *bdev) {struct inode *inode =BD_INODE (bdev);
block/fops.c +1 -0
@@ -941,6 +941,7 @@ const struct file_operations def_blk_fops = { .fallocate = blkdev_fallocate, .uring_cmd = blkdev_uring_cmd, .fop_flags = FOP_BUFFER_RASYNC, + .get_dma_device = block_get_dma_device, };static __init int blkdev_init (void )
include/linux/blk-mq.h +2 -0
@@ -656,6 +656,8 @@ struct blk_mq_ops {*/ void (*map_queues)(struct blk_mq_tag_set *set); +struct device *(*get_dma_device)(struct request_queue *q); +#ifdef CONFIG_BLK_DEBUG_FS /** * @show_rq: Used by the debugfs implementation to show driver-specific
include/linux/blkdev.h +2 -0
@@ -1720,6 +1720,8 @@ struct block_device *file_bdev(struct file *bdev_file);bool disk_live (struct gendisk *disk);unsigned int block_size (struct block_device *bdev); +struct device *block_get_dma_device (struct file *file); +#ifdef CONFIG_BLOCK void invalidate_bdev (struct block_device *bdev);int sync_blockdev (struct block_device *bdev);
Signature · 1 line
2.49.0
RFC 6/12 nvme-pci: add support for user passed dma vectors
In reply to Pavel BegunkovImplement ->get_dma_device blk-mq callback and add BIO_DMAVEC handling. If the drivers see BIO_DMAVEC, instead of mapping pages, it'll directly populate the prp list with the provided dma addresses.
--- drivers/nvme/host/pci.c | 158 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+)
drivers/nvme/host/pci.c +158 -0
@@ -637,11 +637,59 @@ static void nvme_free_descriptors(struct nvme_queue *nvmeq, struct request *req) } } +static void nvme_sync_dma (struct nvme_dev *nvme_dev,struct request *req, +enum dma_data_direction dir) +{ +bool for_cpu = dir ==DMA_FROM_DEVICE ; +struct device *dev = nvme_dev->dev; +struct bio *bio = req->bio; +int offset, length; +struct dmavec *dmav; + +if (!dma_dev_need_sync (dev)) +return ; + + offset = bio->bi_iter.bi_bvec_done; + length =blk_rq_payload_bytes (req); + dmav = &bio->bi_dmavec[bio->bi_iter.bi_idx]; + +while (length) { +u64 dma_addr = dmav->addr + offset; +int dma_len =min (dmav->len - offset, length); + +if (for_cpu) +__dma_sync_single_for_cpu (dev, dma_addr, dma_len, dir); +else +__dma_sync_single_for_device (dev, dma_addr, + dma_len, dir); + + length -= dma_len; + } +} + +static void nvme_unmap_premapped_data (struct nvme_dev *dev, +struct nvme_queue *nvmeq, +struct request *req) +{ +struct nvme_iod *iod =blk_mq_rq_to_pdu (req); + +if (rq_data_dir (req) ==READ ) +nvme_sync_dma (dev, req,DMA_FROM_DEVICE ); + +if (!iod->dma_len) +nvme_free_descriptors (nvmeq, req); +} +static void nvme_unmap_data (struct nvme_dev *dev,struct nvme_queue *nvmeq,struct request *req) {struct nvme_iod *iod =blk_mq_rq_to_pdu (req); +if (req->bio &&bio_flagged (req->bio,BIO_DMAVEC )) { +nvme_unmap_premapped_data (dev, nvmeq, req); +return ; + } +if (iod->dma_len) {dma_unmap_page (dev->dev, iod->first_dma, iod->dma_len,rq_dma_dir (req));
@@ -846,6 +894,104 @@ static blk_status_t nvme_setup_sgl_simple(struct nvme_dev *dev,return BLK_STS_OK ; } +static blk_status_t nvme_dma_premapped (struct nvme_dev *dev,struct request *req, +struct nvme_queue *nvmeq, +struct nvme_rw_command *cmnd) +{ +struct nvme_iod *iod =blk_mq_rq_to_pdu (req); +int length =blk_rq_payload_bytes (req); +u64 dma_addr, first_dma_addr; +struct bio *bio = req->bio; +int dma_len, offset; +struct dmavec *dmav; +dma_addr_t prp_dma; +__le64 *prp_list; +int i; + +if (rq_data_dir (req) ==WRITE ) +nvme_sync_dma (dev, req,DMA_TO_DEVICE ); + + offset = bio->bi_iter.bi_bvec_done; + dmav = &bio->bi_dmavec[bio->bi_iter.bi_idx]; + dma_addr = dmav->addr + offset; + dma_len = dmav->len - offset; + first_dma_addr = dma_addr; + offset = dma_addr & (NVME_CTRL_PAGE_SIZE -1 ); + + length -= (NVME_CTRL_PAGE_SIZE - offset); +if (length <=0 ) { + iod->first_dma =0 ; + goto done; + } + + dma_len -= (NVME_CTRL_PAGE_SIZE - offset); +if (dma_len) { + dma_addr += (NVME_CTRL_PAGE_SIZE - offset); + }else { + dmav++; + dma_addr = dmav->addr; + dma_len = dmav->len; + } + +if (length <=NVME_CTRL_PAGE_SIZE ) { + iod->first_dma = dma_addr; + goto done; + } + +if (DIV_ROUND_UP (length,NVME_CTRL_PAGE_SIZE ) <= +NVME_SMALL_POOL_SIZE /sizeof (__le64)) + iod->flags |=IOD_SMALL_DESCRIPTOR ; + + prp_list =dma_pool_alloc (nvme_dma_pool (nvmeq, iod),GFP_ATOMIC , + &prp_dma); +if (!prp_list) +return BLK_STS_RESOURCE ; + + iod->descriptors[iod->nr_descriptors++] = prp_list; + iod->first_dma = prp_dma; + i =0 ; +for (;;) { +if (i ==NVME_CTRL_PAGE_SIZE >>3 ) { +__le64 *old_prp_list = prp_list; + + prp_list =dma_pool_alloc (nvmeq->descriptor_pools.large, +GFP_ATOMIC , &prp_dma); +if (!prp_list) + goto free_prps; + iod->descriptors[iod->nr_descriptors++] = prp_list; + prp_list[0 ] = old_prp_list[i -1 ]; + old_prp_list[i -1 ] =cpu_to_le64 (prp_dma); + i =1 ; + } + + prp_list[i++] =cpu_to_le64 (dma_addr); + dma_len -=NVME_CTRL_PAGE_SIZE ; + dma_addr +=NVME_CTRL_PAGE_SIZE ; + length -=NVME_CTRL_PAGE_SIZE ; +if (length <=0 ) +break ; +if (dma_len >0 ) +continue ; +if (unlikely (dma_len <0 )) + goto bad_sgl; + dmav++; + dma_addr = dmav->addr; + dma_len = dmav->len; + } +done: + cmnd->dptr.prp1 =cpu_to_le64 (first_dma_addr); + cmnd->dptr.prp2 =cpu_to_le64 (iod->first_dma); +return BLK_STS_OK ; +free_prps: +nvme_free_descriptors (nvmeq, req); +return BLK_STS_RESOURCE ; +bad_sgl: +WARN (DO_ONCE (nvme_print_sgl, iod->sgt.sgl, iod->sgt.nents), +"Invalid SGL for payload:%d nents:%d\n" , +blk_rq_payload_bytes (req), iod->sgt.nents); +return BLK_STS_IOERR ; +} +static blk_status_t nvme_map_data (struct nvme_dev *dev,struct request *req,struct nvme_command *cmnd) {
@@ -854,6 +1000,9 @@ static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req,blk_status_t ret =BLK_STS_RESOURCE ;int rc; +if (req->bio &&bio_flagged (req->bio,BIO_DMAVEC )) +return nvme_dma_premapped (dev, req, nvmeq, &cmnd->rw); +if (blk_rq_nr_phys_segments (req) ==1 ) {struct bio_vec bv =req_bvec (req);
@@ -1874,6 +2023,14 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid, bool polled)return result; } +static struct device *nvme_pci_get_dma_device (struct request_queue *q) +{ +struct nvme_ns *ns = q->queuedata; +struct nvme_dev *dev =to_nvme_dev (ns->ctrl); + +return dev->dev; +} +static const struct blk_mq_ops nvme_mq_admin_ops = { .queue_rq = nvme_queue_rq, .complete = nvme_pci_complete_rq,
@@ -1892,6 +2049,7 @@ static const struct blk_mq_ops nvme_mq_ops = { .map_queues = nvme_pci_map_queues, .timeout = nvme_timeout, .poll = nvme_poll, + .get_dma_device = nvme_pci_get_dma_device, };static void nvme_dev_remove_admin (struct nvme_dev *dev)
Signature · 1 line
2.49.0
RFC 7/12 io_uring/rsrc: extended reg buffer registration
In reply to Pavel BegunkovWe'll need to pass extra information for buffer registration apart from iovec, add a flag to struct io_uring_rsrc_update2 that tells that its data fields points to an extended registration structure, i.e. struct io_uring_reg_buffer. To do normal registration the user has to set target_fd and dmabuf_fd fields to -1, and any other combination is currently rejected.
--- include/uapi/linux/io_uring.h | 13 ++++++++- io_uring/rsrc.c | 53 +++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 12 deletions(-)
include/uapi/linux/io_uring.h +12 -1
@@ -725,15 +725,26 @@ struct io_uring_rsrc_update {__aligned_u64 data; }; +/* struct io_uring_rsrc_update2::flags */ +enum io_uring_rsrc_reg_flags { +IORING_RSRC_F_EXTENDED_UPDATE =1 , +}; +struct io_uring_rsrc_update2 {__u32 offset; -__u32 resv; +__u32 flags;__aligned_u64 data;__aligned_u64 tags;__u32 nr;__u32 resv2; }; +struct io_uring_reg_buffer { +__aligned_u64 iov_uaddr; +__s32 target_fd; +__s32 dmabuf_fd; +}; +/* Skip updating fd indexes set to this value in the fd table */ #define IORING_REGISTER_FILES_SKIP (-2)
io_uring/rsrc.c +42 -11
142 lines not on this page · raw message
Signature · 1 line
2.49.0
RFC 8/12 io_uring: add basic dmabuf helpers
In reply to Pavel BegunkovAdd basic dmabuf helpers and structures for io_uring, which will be used for dmabuf buffer registration. That can also be reused in other places in io_uring, which is ommited from the series.
--- io_uring/Makefile | 1 + io_uring/dmabuf.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++ io_uring/dmabuf.h | 34 +++++++++++++++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 io_uring/dmabuf.c create mode 100644 io_uring/dmabuf.h
io_uring/Makefile +1 -0
@@ -21,3 +21,4 @@ obj-$(CONFIG_EPOLL) += epoll.o obj-$(CONFIG_NET_RX_BUSY_POLL) += napi.o obj-$(CONFIG_NET) += net.o cmd_net.o obj-$(CONFIG_PROC_FS) += fdinfo.o +obj-$(CONFIG_DMA_SHARED_BUFFER) += dmabuf.o
io_uring/dmabuf.c new file +60 -0
60 lines not on this page · raw message
io_uring/dmabuf.h new file +34 -0
@@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0 +#ifndef IOU_DMABUF_H +#define IOU_DMABUF_H + +#include <linux/io_uring_types.h> +#include <linux/dma-buf.h> + +struct io_dmabuf { +size_t len; +struct dma_buf_attachment *attach; +struct dma_buf *dmabuf; +struct sg_table *sgt; +struct device *dev; +enum dma_data_direction dir; +}; + +#ifdef CONFIG_DMA_SHARED_BUFFER +void io_dmabuf_release (struct io_dmabuf *buf); +int io_dmabuf_import (struct io_dmabuf *buf,int dmabuf_fd, +struct device *dev,enum dma_data_direction dir); + +#else +static inline void io_dmabuf_release (struct io_dmabuf *buf) +{ +} + +static inline int io_dmabuf_import (struct io_dmabuf *buf,int dmabuf_fd, +struct device *dev,enum dma_data_direction dir) +{ +return -EOPNOTSUPP ; +} +#endif + +#endif
Signature · 1 line
2.49.0
RFC 9/12 io_uring/rsrc: add imu flags
In reply to Pavel BegunkovReplace is_kbuf with a flags field in io_mapped_ubuf. There will be new flags shortly, and bit fields are often not as convenient to work with.
--- io_uring/rsrc.c | 12 ++++++------ io_uring/rsrc.h | 6 +++++- io_uring/rw.c | 3 ++- 3 files changed, 13 insertions(+), 8 deletions(-)
io_uring/rsrc.c +6 -6
@@ -850,7 +850,7 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx, imu->folio_shift =PAGE_SHIFT ; imu->release = io_release_ubuf; imu->priv = imu; - imu->is_kbuf = false; + imu->flags =0 ; imu->dir =IO_IMU_DEST |IO_IMU_SOURCE ;if (coalesced) imu->folio_shift = data.folio_shift;
@@ -999,7 +999,7 @@ int io_buffer_register_bvec(struct io_uring_cmd *cmd, struct request *rq,refcount_set (&imu->refs,1 ); imu->release = release; imu->priv = rq; - imu->is_kbuf = true; + imu->flags =IO_IMU_F_KBUF ; imu->dir =1 <<rq_data_dir (rq); bvec = imu->bvec;
@@ -1034,7 +1034,7 @@ int io_buffer_unregister_bvec(struct io_uring_cmd *cmd, unsigned int index, ret = -EINVAL ; goto unlock; } -if (!node->buf->is_kbuf) { +if (!(node->buf->flags &IO_IMU_F_KBUF )) { ret = -EBUSY ; goto unlock; }
@@ -1100,7 +1100,7 @@ static int io_import_fixed(int ddir, struct iov_iter *iter, offset = buf_addr - imu->ubuf; -if (imu->is_kbuf) +if (imu->flags &IO_IMU_F_KBUF )return io_import_kbuf (ddir, iter, imu, len, offset);/*
@@ -1509,7 +1509,7 @@ int io_import_reg_vec(int ddir, struct iov_iter *iter, iovec_off = vec->nr - nr_iovs; iov = vec->iovec + iovec_off; -if (imu->is_kbuf) { +if (imu->flags &IO_IMU_F_KBUF ) {int ret =io_kern_bvec_size (iov, nr_iovs, imu, &nr_segs);if (unlikely (ret))
@@ -1543,7 +1543,7 @@ int io_import_reg_vec(int ddir, struct iov_iter *iter, req->flags |=REQ_F_NEED_CLEANUP ; } -if (imu->is_kbuf) +if (imu->flags &IO_IMU_F_KBUF )return io_vec_fill_kern_bvec (ddir, iter, imu, iov, nr_iovs, vec);return io_vec_fill_bvec (ddir, iter, imu, iov, nr_iovs, vec);
io_uring/rsrc.h +5 -1
@@ -28,6 +28,10 @@ enum { IO_IMU_SOURCE = 1 << ITER_SOURCE, }; +enum { +IO_IMU_F_KBUF =1 , +}; +struct io_mapped_ubuf {u64 ubuf;unsigned int len;
@@ -37,7 +41,7 @@ struct io_mapped_ubuf {unsigned long acct_pages;void (*release)(void *);void *priv; -bool is_kbuf; +u8 flags;u8 dir; struct bio_vec bvec[] __counted_by(nr_bvecs); };
io_uring/rw.c +2 -1
@@ -696,7 +696,8 @@ static ssize_t loop_rw_iter(int ddir, struct io_rw *rw, struct iov_iter *iter)if ((kiocb->ki_flags &IOCB_NOWAIT ) && !(kiocb->ki_filp->f_flags &O_NONBLOCK ))return -EAGAIN ; -if ((req->flags &REQ_F_BUF_NODE ) && req->buf_node->buf->is_kbuf) +if ((req->flags &REQ_F_BUF_NODE ) && + (req->buf_node->buf->flags &IO_IMU_F_KBUF ))return -EFAULT ; ppos =io_kiocb_ppos (kiocb);
Signature · 1 line
2.49.0
RFC 10/12 io_uring/rsrc: add dmabuf-backed buffer registeration
In reply to Pavel BegunkovAdd an ability to register a dmabuf backed io_uring buffer. It also needs know which device to use for attachment, for that it takes target_fd and extracts the device through the new file op. Unlike normal buffers, it also retains the target file so that any imports from ineligible requests can be rejected in next patches.
--- io_uring/rsrc.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++- io_uring/rsrc.h | 1 + 2 files changed, 118 insertions(+), 1 deletion(-)
io_uring/rsrc.c +117 -1
@@ -10,6 +10,8 @@#include <linux/compat.h> #include <linux/io_uring.h> #include <linux/io_uring/cmd.h> +#include <linux/dma-map-ops.h> +#include <linux/dma-buf.h> #include <uapi/linux/io_uring.h>
@@ -18,6 +20,7 @@#include "rsrc.h" #include "memmap.h" #include "register.h" +#include "dmabuf.h" struct io_rsrc_update {struct file *file;
@@ -793,6 +796,117 @@ bool io_check_coalesce_buffer(struct page **page_array, int nr_pages,return true; } +struct io_regbuf_dma { +struct io_dmabuf dmabuf; +struct dmavec *dmav; +struct file *target_file; +}; + +static void io_release_reg_dmabuf (struct io_regbuf_dma *db) +{ +if (db->dmav) +kfree (db->dmav); +io_dmabuf_release (&db->dmabuf); +if (db->target_file) +fput (db->target_file); + +kfree (db); +} + +static void io_release_reg_dmabuf_cb (void *priv) +{ +io_release_reg_dmabuf (priv); +} + +static struct io_rsrc_node *io_register_dmabuf (struct io_ring_ctx *ctx, +struct io_uring_reg_buffer *rb, +struct iovec *iov) +{ +struct io_rsrc_node *node =NULL ; +struct io_mapped_ubuf *imu =NULL ; +struct io_regbuf_dma *regbuf; +struct file *target_file; +struct scatterlist *sg; +struct device *dev; +unsigned int segments; +int ret, i; + +if (iov->iov_base || iov->iov_len) +return ERR_PTR (-EFAULT ); + + regbuf =kzalloc (sizeof (*regbuf),GFP_KERNEL ); +if (!regbuf) { + ret = -ENOMEM ; + goto err; + } + + target_file =fget (rb->target_fd); +if (!target_file) { + ret = -EBADF ; + goto err; + } + regbuf->target_file = target_file; + +if (!target_file->f_op->get_dma_device) { + ret = -EOPNOTSUPP ; + goto err; + } + dev = target_file->f_op->get_dma_device (target_file); +if (IS_ERR (dev)) { + ret =PTR_ERR (dev); + goto err; + } + + ret =io_dmabuf_import (®buf->dmabuf, rb->dmabuf_fd, dev, +DMA_BIDIRECTIONAL ); +if (ret) + goto err; + + segments = regbuf->dmabuf.sgt->nents; + regbuf->dmav =kmalloc_array (segments,sizeof (regbuf->dmav[0 ]), +GFP_KERNEL_ACCOUNT ); +if (!regbuf->dmav) { + ret = -ENOMEM ; + goto err; + } + +for_each_sgtable_dma_sg (regbuf->dmabuf.sgt, sg, i) { + regbuf->dmav[i].addr =sg_dma_address (sg); + regbuf->dmav[i].len =sg_dma_len (sg); + } + + node =io_rsrc_node_alloc (ctx,IORING_RSRC_BUFFER ); +if (!node) { + ret = -ENOMEM ; + goto err; + } + imu =io_alloc_imu (ctx,0 ); +if (!imu) { + ret = -ENOMEM ; + goto err; + } + + imu->nr_bvecs = segments; + imu->ubuf =0 ; + imu->len = regbuf->dmabuf.len; + imu->folio_shift =0 ; + imu->release = io_release_reg_dmabuf_cb; + imu->priv = regbuf; + imu->flags =IO_IMU_F_DMA ; + imu->dir =IO_IMU_DEST |IO_IMU_SOURCE ; +refcount_set (&imu->refs,1 ); + node->buf = imu; +return node; +err: +if (regbuf) +io_release_reg_dmabuf (regbuf); +if (imu) +io_free_imu (ctx, imu); +if (node) +io_cache_free (&ctx->node_cache, node); +return ERR_PTR (ret); +} + static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx, struct io_uring_reg_buffer *rb, struct iovec *iov,
@@ -808,7 +922,7 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx,bool coalesced = false;if (rb->dmabuf_fd !=-1 || rb->target_fd !=-1 ) -return NULL ; +return io_register_dmabuf (ctx, rb, iov);if (!iov->iov_base)return NULL ;
@@ -1100,6 +1214,8 @@ static int io_import_fixed(int ddir, struct iov_iter *iter, offset = buf_addr - imu->ubuf; +if (imu->flags &IO_IMU_F_DMA ) +return -EOPNOTSUPP ;if (imu->flags &IO_IMU_F_KBUF )return io_import_kbuf (ddir, iter, imu, len, offset);
io_uring/rsrc.h +1 -0
@@ -30,6 +30,7 @@ enum {enum {IO_IMU_F_KBUF =1 , +IO_IMU_F_DMA =2 , };struct io_mapped_ubuf {
Signature · 1 line
2.49.0
RFC 11/12 io_uring/rsrc: implement dmabuf regbuf import
In reply to Pavel BegunkovAllow importing dmabuf backed registered buffers. It's an opt-in feature for requests and they need to pass a flag allowing it. Furthermore, the import will fail if the request's file doesn't match the file for which the buffer for registered. This way, it's also limited to files that support the feature by implementing the corresponding file op.
--- io_uring/rsrc.c | 53 ++++++++++++++++++++++++++++++++++++++++++------- io_uring/rsrc.h | 16 ++++++++++++++- 2 files changed, 61 insertions(+), 8 deletions(-)
io_uring/rsrc.c +46 -7
@@ -1196,9 +1196,44 @@ static int io_import_kbuf(int ddir, struct iov_iter *iter,return 0 ; } -static int io_import_fixed (int ddir,struct iov_iter *iter, +static int io_import_dmabuf (struct io_kiocb *req, +int ddir,struct iov_iter *iter,struct io_mapped_ubuf *imu, -u64 buf_addr,size_t len) +size_t len,size_t offset) +{ +struct io_regbuf_dma *db = imu->priv; +struct dmavec *dmavec = db->dmav; +int i =0 , start_idx, nr_segs; +ssize_t len_left; + +if (req->file != db->target_file) +return -EBADF ; +if (!len) +return -EFAULT ; + +while (offset >= dmavec[i].len) { + offset -= dmavec[i].len; + i++; + } + start_idx = i; + + len_left = len; +while (len_left >0 ) { + len_left -= dmavec[i].len; + i++; + } + + nr_segs = i - start_idx; +iov_iter_dma (iter, ddir, dmavec + start_idx, nr_segs, len); + iter->iov_offset = offset; +return 0 ; +} + +static int io_import_fixed (struct io_kiocb *req, +int ddir,struct iov_iter *iter, +struct io_mapped_ubuf *imu, +u64 buf_addr,size_t len, +unsigned import_flags) {const struct bio_vec *bvec;size_t folio_mask;
@@ -1214,8 +1249,11 @@ static int io_import_fixed(int ddir, struct iov_iter *iter, offset = buf_addr - imu->ubuf; -if (imu->flags &IO_IMU_F_DMA ) -return -EOPNOTSUPP ; +if (imu->flags &IO_IMU_F_DMA ) { +if (!(import_flags &IO_REGBUF_IMPORT_ALLOW_DMA )) +return -EFAULT ; +return io_import_dmabuf (req, ddir, iter, imu, len, offset); + }if (imu->flags &IO_IMU_F_KBUF )return io_import_kbuf (ddir, iter, imu, len, offset);
@@ -1269,16 +1307,17 @@ inline struct io_rsrc_node *io_find_buf_node(struct io_kiocb *req,return NULL ; } -int io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter, +int __io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter,u64 buf_addr,size_t len,int ddir, -unsigned issue_flags) +unsigned issue_flags,unsigned import_flags) {struct io_rsrc_node *node; node =io_find_buf_node (req, issue_flags);if (!node)return -EFAULT ; -return io_import_fixed (ddir, iter, node->buf, buf_addr, len); +return io_import_fixed (req, ddir, iter, node->buf, buf_addr, len, + import_flags); }/* Lock two rings at once. The rings must be different! */
io_uring/rsrc.h +15 -1
@@ -33,6 +33,10 @@ enum { IO_IMU_F_DMA = 2, }; +enum { +IO_REGBUF_IMPORT_ALLOW_DMA =1 , +}; +struct io_mapped_ubuf {u64 ubuf;unsigned int len;
@@ -65,9 +69,19 @@ int io_rsrc_data_alloc(struct io_rsrc_data *data, unsigned nr);struct io_rsrc_node *io_find_buf_node (struct io_kiocb *req,unsigned issue_flags); +int __io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter, +u64 buf_addr,size_t len,int ddir, +unsigned issue_flags,unsigned import_flags); + +static inline int io_import_reg_buf (struct io_kiocb *req,struct iov_iter *iter,u64 buf_addr,size_t len,int ddir, -unsigned issue_flags); +unsigned issue_flags) +{ +return __io_import_reg_buf (req, iter, buf_addr, len, ddir, + issue_flags,0 ); +} +int io_import_reg_vec (int ddir,struct iov_iter *iter,struct io_kiocb *req,struct iou_vec *vec,unsigned nr_iovs,unsigned issue_flags);
Signature · 1 line
2.49.0
RFC 12/12 io_uring/rw: enable dma registered buffers
In reply to Pavel BegunkovEnable dmabuf registered buffer from the read-write path.
--- io_uring/rw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
io_uring/rw.c +2 -2
@@ -372,8 +372,8 @@ static int io_init_rw_fixed(struct io_kiocb *req, unsigned int issue_flags,if (io->bytes_done)return 0 ; - ret =io_import_reg_buf (req, &io->iter, rw->addr, rw->len, ddir, - issue_flags); + ret =__io_import_reg_buf (req, &io->iter, rw->addr, rw->len, ddir, + issue_flags,IO_REGBUF_IMPORT_ALLOW_DMA );iov_iter_save_state (&io->iter, &io->iter_state);return ret; }
Signature · 1 line
2.49.0
[Note: it would be really useful to Cc all relevant maintainers]
Quoted 5 lines · Pavel Begunkov
This series implements it for read/write
io_uringrequests. The uAPI looks similar to normal registered buffers, the user will need to register a dmabuf inio_uringfirst and then use it as any other registered buffer. On registration the user also specifies a file to map the dmabuf for.
Just commenting from the in-kernel POV here, where the interface feels wrong.
You can't just expose 'the DMA device' up file operations, because there can be and often is more than one. Similarly stuffing a dma_addr_t into an iovec is rather dangerous.
The model that should work much better is to have file operations to attach to / detach from a dma_buf, and then have an iter that specifies a dmabuf and offsets into. That way the code behind the file operations can forward the attachment to all the needed devices (including more/less while it remains attached to the file) and can pick the right dma address for each device.
I also remember some discussion that new dma-buf importers should use the dynamic imported model for long-term imports, but as I'm everything but an expert in that area I'll let the dma-buf folks speak.
Quoted 23 lines · Christoph Hellwig
[Note: it would be really useful to Cc all relevant maintainers]
Quoted 5 lines · Pavel Begunkov
This series implements it for read/write
io_uringrequests. The uAPI looks similar to normal registered buffers, the user will need to register a dmabuf inio_uringfirst and then use it as any other registered buffer. On registration the user also specifies a file to map the dmabuf for.Just commenting from the in-kernel POV here, where the interface feels wrong.
You can't just expose 'the DMA device' up file operations, because there can be and often is more than one. Similarly stuffing a
dma_addr_tinto an iovec is rather dangerous.The model that should work much better is to have file operations to attach to / detach from a
dma_buf, and then have an iter that specifies a dmabuf and offsets into. That way the code behind the file operations can forward the attachment to all the needed devices (including more/less while it remains attached to the file) and can pick the right dma address for each device.
Quoted 4 lines
I also remember some discussion that new dma-buf importers should use the dynamic imported model for long-term imports, but as I'm everything but an expert in that area I'll let the dma-buf folks speak.
Completely correct.
As long as you don't have a really good explanation and some mechanism to prevent abuse long term pinning of DMA-bufs should be avoided.
Regards,
Christian.
Quoted 1 line · Christoph Hellwig
[Note: it would be really useful to Cc all relevant maintainers]
Will do next time
Quoted 14 lines · Pavel Begunkov
Quoted 5 lines
This series implements it for read/write
io_uringrequests. The uAPI looks similar to normal registered buffers, the user will need to register a dmabuf inio_uringfirst and then use it as any other registered buffer. On registration the user also specifies a file to map the dmabuf for.Just commenting from the in-kernel POV here, where the interface feels wrong.
You can't just expose 'the DMA device' up file operations, because there can be and often is more than one. Similarly stuffing a
dma_addr_tinto an iovec is rather dangerous.
Quoted 6 lines
The model that should work much better is to have file operations to attach to / detach from a
dma_buf, and then have an iter that specifies a dmabuf and offsets into. That way the code behind the file operations can forward the attachment to all the needed devices (including more/less while it remains attached to the file) and can pick the right dma address for each device.
By "iter that specifies a dmabuf" do you mean an opaque file-specific structure allocated inside the new fop? Akin to what Keith proposed back then. That sounds good and has more potential for various optimisations. My concern would be growing struct iov_iter by an extra pointer:
struct dma_seg {
size_t off;
unsigned len;
};struct iov_iter {
union {
struct iovec *iov;
struct dma_seg *dmav;
...
};
void *dma_token;
};But maybe that's fine. It's 40B -> 48B, and it'll get back to 40 when / if xarray_start / ITER_XARRAY is removed.
Quoted 4 lines
I also remember some discussion that new dma-buf importers should use the dynamic imported model for long-term imports, but as I'm everything but an expert in that area I'll let the dma-buf folks speak.
I'll take a look
Signature · 1 line
Pavel Begunkov
Quoted 8 lines · Pavel Begunkov
Quoted 5 lines
to attach to / detach from a
dma_buf, and then have an iter that specifies a dmabuf and offsets into. That way the code behind the file operations can forward the attachment to all the needed devices (including more/less while it remains attached to the file) and can pick the right dma address for each device.By "iter that specifies a dmabuf" do you mean an opaque file-specific structure allocated inside the new fop?
I mean a reference the actual dma_buf (probably indirect through the file
* for it, but listen to the dma_buf experts for that and not me).
Quoted 13 lines
Akin to what Keith proposed back
then. That sounds good and has more potential for various optimisations. My concern would be growing structiov_iterby an extra pointer:struct iov_iter { union { struct iovec *iov; struct dma_seg *dmav; ... }; void *dma_token; };
Quoted 1 line
But maybe that's fine. It's 40B -> 48B,
Alternatively we could the union point to a struct that has the dma buf pointer and a variable length array of dma_segs. Not sure if that would create a mess in the callers, though.
Quoted 2 lines
and it'll get back to
40 when / ifxarray_start/ITER_XARRAYis removed.
Would it? At least for 64-bit architectures nr_segs is the same size.
Quoted 7 lines · Christoph Hellwig
Quoted 6 lines · Pavel Begunkov
Quoted 5 lines
to attach to / detach from a
dma_buf, and then have an iter that specifies a dmabuf and offsets into. That way the code behind the file operations can forward the attachment to all the needed devices (including more/less while it remains attached to the file) and can pick the right dma address for each device.
Quoted 5 lines
Quoted 2 lines
By "iter that specifies a dmabuf" do you mean an opaque file-specific structure allocated inside the new fop?
I mean a reference the actual
dma_buf(probably indirect through the file
* for it, but listen to thedma_bufexperts for that and not me).
My expectation is that io_uring would pass struct dma_buf to the file during registration, so that it can do a bunch of work upfront, but iterators will carry sth already pre-attached and pre dma mapped, probably in a file specific format hiding details for multi-device support, and possibly bundled with the dma-buf pointer if necessary. (All modulo move notify which I need to look into first).
Quoted 13 lines
Quoted 13 lines
Akin to what Keith proposed back
then. That sounds good and has more potential for various optimisations. My concern would be growing structiov_iterby an extra pointer:struct iov_iter { union { struct iovec *iov; struct dma_seg *dmav; ... }; void *dma_token; };
Quoted 5 lines
Quoted 1 line
But maybe that's fine. It's 40B -> 48B,
Alternatively we could the union point to a struct that has the dma buf pointer and a variable length array of
dma_segs. Not sure if that would create a mess in the callers, though.
Iteration helpers adjust the pointer, so either it needs to store the pointer directly in iter or keep the current index. It could rely solely on offsets, but that'll be a mess with nested loops (where the inner one would walk some kind of sg table).
Quoted 4 lines
Quoted 2 lines
and it'll get back to
40 when / ifxarray_start/ITER_XARRAYis removed.Would it? At least for 64-bit architectures
nr_segsis the same size.
Ah yes
Signature · 1 line
Pavel Begunkov
Quoted 4 lines · Pavel Begunkov
Quoted 2 lines
I mean a reference the actual
dma_buf(probably indirect through the file
* for it, but listen to thedma_bufexperts for that and not me).My expectation is that
io_uringwould pass structdma_bufto the
io_uring isn't the only user. We've already had one other use case coming up for pre-load of media files in mobile very recently. It's also a really good interface for P2P transfers of any kind.
Quoted 5 lines
file during registration, so that it can do a bunch of work upfront, but iterators will carry sth already pre-attached and pre dma mapped, probably in a file specific format hiding details for multi-device support, and possibly bundled with the dma-buf pointer if necessary. (All modulo move notify which I need to look into first).
I'd expect that the exported passed around the dma_buf, and something that has access to it then imports it to the file. This could be directly forwarded to the device for the initial scrope in your series where you only support it for block device files.
Now we have two variants:
1) the file instance returns a cookie for the registration that the caller has to pass into every read/write
2) the file instance tracks said cookie itself and matches it on every read/write
1) sounds faster, 2) has more sanity checking and could prevent things from going wrong.
(all this is based on my limited dma_buf understanding, corrections always welcome).
Quoted 10 lines
Quoted 5 lines
Quoted 1 line
But maybe that's fine. It's 40B -> 48B,
Alternatively we could the union point to a struct that has the dma buf pointer and a variable length array of
dma_segs. Not sure if that would create a mess in the callers, though.Iteration helpers adjust the pointer, so either it needs to store the pointer directly in iter or keep the current index. It could rely solely on offsets, but that'll be a mess with nested loops (where the inner one would walk some kind of sg table).
Yeah. Maybe just keep is as a separate pointer growing the structure and see if anyone screams.