Re: [PATCH v4 19/25] fuse: add io-uring kernel-managed buffer ring

[Bernd Schubert <bernd@bsbernd.com>]

On 1/28/26 22:44, Bernd Schubert wrote:
> 
> 
> On 1/17/26 00:30, Joanne Koong wrote:
>> Add io-uring kernel-managed buffer ring capability for fuse daemons
>> communicating through the io-uring interface.
>>
>> This has two benefits:
>> a) eliminates the overhead of pinning/unpinning user pages and
>> translating virtual addresses for every server-kernel interaction
>>
>> b) reduces the amount of memory needed for the buffers per queue and
>> allows buffers to be reused across entries. Incremental buffer
>> consumption, when added, will allow a buffer to be used across multiple
>> requests.
>>
>> Buffer ring usage is set on a per-queue basis. In order to use this, the
>> daemon needs to have preregistered a kernel-managed buffer ring and a
>> fixed buffer at index 0 that will hold all the headers, and set the
>> "use_bufring" field during registration. The kernel-managed buffer ring
>> will be pinned for the lifetime of the connection.
>>
>> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
>> ---
>>  fs/fuse/dev_uring.c       | 412 ++++++++++++++++++++++++++++++++------
>>  fs/fuse/dev_uring_i.h     |  31 ++-
>>  include/uapi/linux/fuse.h |  15 +-
>>  3 files changed, 389 insertions(+), 69 deletions(-)
>>
>> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
>> index b57871f92d08..40e8c2e6b77c 100644
>> --- a/fs/fuse/dev_uring.c
>> +++ b/fs/fuse/dev_uring.c
>> @@ -10,6 +10,8 @@
>>  #include "fuse_trace.h"
>>  
>>  #include <linux/fs.h>
>> +#include <linux/io_uring.h>
>> +#include <linux/io_uring/buf.h>
>>  #include <linux/io_uring/cmd.h>
>>  
>>  static bool __read_mostly enable_uring;
>> @@ -19,6 +21,8 @@ MODULE_PARM_DESC(enable_uring,
>>  
>>  #define FUSE_URING_IOV_SEGS 2 /* header and payload */
>>  
>> +#define FUSE_URING_RINGBUF_GROUP 0
>> +#define FUSE_URING_FIXED_HEADERS_OFFSET 0
>>  
>>  bool fuse_uring_enabled(void)
>>  {
>> @@ -276,20 +280,45 @@ static struct fuse_ring *fuse_uring_create(struct fuse_conn *fc)
>>  	return res;
>>  }
>>  
>> -static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
>> -						       int qid)
>> +static int fuse_uring_buf_ring_setup(struct io_uring_cmd *cmd,
>> +				     struct fuse_ring_queue *queue,
>> +				     unsigned int issue_flags)
>> +{
>> +	int err;
>> +
>> +	err = io_uring_buf_ring_pin(cmd, FUSE_URING_RINGBUF_GROUP, issue_flags,
>> +				    &queue->bufring);
>> +	if (err)
>> +		return err;
>> +
>> +	if (!io_uring_is_kmbuf_ring(cmd, FUSE_URING_RINGBUF_GROUP,
>> +				    issue_flags)) {
>> +		io_uring_buf_ring_unpin(cmd, FUSE_URING_RINGBUF_GROUP,
>> +					issue_flags);
>> +		return -EINVAL;
>> +	}
>> +
>> +	queue->use_bufring = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static struct fuse_ring_queue *
>> +fuse_uring_create_queue(struct io_uring_cmd *cmd, struct fuse_ring *ring,
>> +			int qid, bool use_bufring, unsigned int issue_flags)
>>  {
>>  	struct fuse_conn *fc = ring->fc;
>>  	struct fuse_ring_queue *queue;
>>  	struct list_head *pq;
>> +	int err;
>>  
>>  	queue = kzalloc(sizeof(*queue), GFP_KERNEL_ACCOUNT);
>>  	if (!queue)
>> -		return NULL;
>> +		return ERR_PTR(-ENOMEM);
>>  	pq = kcalloc(FUSE_PQ_HASH_SIZE, sizeof(struct list_head), GFP_KERNEL);
>>  	if (!pq) {
>>  		kfree(queue);
>> -		return NULL;
>> +		return ERR_PTR(-ENOMEM);
>>  	}
>>  
>>  	queue->qid = qid;
>> @@ -307,6 +336,15 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
>>  	queue->fpq.processing = pq;
>>  	fuse_pqueue_init(&queue->fpq);
>>  
>> +	if (use_bufring) {
>> +		err = fuse_uring_buf_ring_setup(cmd, queue, issue_flags);
>> +		if (err) {
>> +			kfree(pq);
>> +			kfree(queue);
>> +			return ERR_PTR(err);
>> +		}
>> +	}
>> +
>>  	spin_lock(&fc->lock);
>>  	if (ring->queues[qid]) {
>>  		spin_unlock(&fc->lock);
>> @@ -584,6 +622,35 @@ static int fuse_uring_out_header_has_err(struct fuse_out_header *oh,
>>  	return err;
>>  }
>>  
>> +static int get_kernel_ring_header(struct fuse_ring_ent *ent,
>> +				  enum fuse_uring_header_type type,
>> +				  struct iov_iter *headers_iter)
>> +{
>> +	size_t offset;
>> +
>> +	switch (type) {
>> +	case FUSE_URING_HEADER_IN_OUT:
>> +		/* No offset - start of header */
>> +		offset = 0;
>> +		break;
>> +	case FUSE_URING_HEADER_OP:
>> +		offset = offsetof(struct fuse_uring_req_header, op_in);
>> +		break;
>> +	case FUSE_URING_HEADER_RING_ENT:
>> +		offset = offsetof(struct fuse_uring_req_header, ring_ent_in_out);
>> +		break;
>> +	default:
>> +		WARN_ONCE(1, "Invalid header type: %d\n", type);
>> +		return -EINVAL;
>> +	}
>> +
>> +	*headers_iter = ent->headers_iter;
>> +	if (offset)
>> +		iov_iter_advance(headers_iter, offset);
>> +
>> +	return 0;
>> +}
>> +
>>  static void __user *get_user_ring_header(struct fuse_ring_ent *ent,
>>  					 enum fuse_uring_header_type type)
>>  {
>> @@ -605,17 +672,38 @@ static __always_inline int copy_header_to_ring(struct fuse_ring_ent *ent,
>>  					       const void *header,
>>  					       size_t header_size)
>>  {
>> -	void __user *ring = get_user_ring_header(ent, type);
>> +	bool use_bufring = ent->queue->use_bufring;
>> +	int err = 0;
>>  
>> -	if (!ring)
>> -		return -EINVAL;
>> +	if (use_bufring) {
>> +		struct iov_iter iter;
>> +
>> +		err =  get_kernel_ring_header(ent, type, &iter);
>> +		if (err)
>> +			goto done;
>> +
>> +		if (copy_to_iter(header, header_size, &iter) != header_size)
>> +			err = -EFAULT;
>> +	} else {
>> +		void __user *ring = get_user_ring_header(ent, type);
>> +
>> +		if (!ring) {
>> +			err = -EINVAL;
>> +			goto done;
>> +		}
>>  
>> -	if (copy_to_user(ring, header, header_size)) {
>> -		pr_info_ratelimited("Copying header to ring failed.\n");
>> -		return -EFAULT;
>> +		if (copy_to_user(ring, header, header_size))
>> +			err = -EFAULT;
>>  	}
>>  
>> -	return 0;
>> +done:
>> +	if (err)
>> +		pr_info_ratelimited("Copying header to ring failed: "
>> +				    "header_type=%u, header_size=%zu, "
>> +				    "use_bufring=%d\n", type, header_size,
>> +				    use_bufring);
>> +
>> +	return err;
>>  }
>>  
>>  static __always_inline int copy_header_from_ring(struct fuse_ring_ent *ent,
>> @@ -623,17 +711,38 @@ static __always_inline int copy_header_from_ring(struct fuse_ring_ent *ent,
>>  						 void *header,
>>  						 size_t header_size)
>>  {
>> -	const void __user *ring = get_user_ring_header(ent, type);
>> +	bool use_bufring = ent->queue->use_bufring;
>> +	int err = 0;
>>  
>> -	if (!ring)
>> -		return -EINVAL;
>> +	if (use_bufring) {
>> +		struct iov_iter iter;
>> +
>> +		err = get_kernel_ring_header(ent, type, &iter);
>> +		if (err)
>> +			goto done;
>> +
>> +		if (copy_from_iter(header, header_size, &iter) != header_size)
>> +			err = -EFAULT;
>> +	} else {
>> +		const void __user *ring = get_user_ring_header(ent, type);
>>  
>> -	if (copy_from_user(header, ring, header_size)) {
>> -		pr_info_ratelimited("Copying header from ring failed.\n");
>> -		return -EFAULT;
>> +		if (!ring) {
>> +			err = -EINVAL;
>> +			goto done;
>> +		}
>> +
>> +		if (copy_from_user(header, ring, header_size))
>> +			err = -EFAULT;
>>  	}
>>  
>> -	return 0;
>> +done:
>> +	if (err)
>> +		pr_info_ratelimited("Copying header from ring failed: "
>> +				    "header_type=%u, header_size=%zu, "
>> +				    "use_bufring=%d\n", type, header_size,
>> +				    use_bufring);
>> +
>> +	return err;
>>  }
>>  
>>  static int setup_fuse_copy_state(struct fuse_copy_state *cs,
>> @@ -643,14 +752,23 @@ static int setup_fuse_copy_state(struct fuse_copy_state *cs,
>>  {
>>  	int err;
>>  
>> -	err = import_ubuf(dir, ent->payload, ring->max_payload_sz, iter);
>> -	if (err) {
>> -		pr_info_ratelimited("fuse: Import of user buffer failed\n");
>> -		return err;
>> +	if (!ent->queue->use_bufring) {
>> +		err = import_ubuf(dir, ent->payload, ring->max_payload_sz, iter);
>> +		if (err) {
>> +			pr_info_ratelimited("fuse: Import of user buffer "
>> +					    "failed\n");
>> +			return err;
>> +		}
>>  	}
>>  
>>  	fuse_copy_init(cs, dir == ITER_DEST, iter);
>>  
>> +	if (ent->queue->use_bufring) {
>> +		cs->is_kaddr = true;
>> +		cs->len = ent->payload_kvec.iov_len;
>> +		cs->kaddr = ent->payload_kvec.iov_base;
>> +	}
>> +
>>  	cs->is_uring = true;
>>  	cs->req = req;
>>  
>> @@ -762,6 +880,94 @@ static int fuse_uring_copy_to_ring(struct fuse_ring_ent *ent,
>>  				   sizeof(req->in.h));
>>  }
>>  
>> +static bool fuse_uring_req_has_payload(struct fuse_req *req)
>> +{
>> +	struct fuse_args *args = req->args;
>> +
>> +	return args->in_numargs > 1 || args->out_numargs;
>> +}
>> +
>> +static int fuse_uring_select_buffer(struct fuse_ring_ent *ent,
>> +				    unsigned int issue_flags)
>> +	__must_hold(&queue->lock)
>> +{
>> +	struct io_br_sel sel;
>> +	size_t len = 0;
>> +
>> +	lockdep_assert_held(&ent->queue->lock);
>> +
>> +	/* Get a buffer to use for the payload */
>> +	sel = io_ring_buffer_select(cmd_to_io_kiocb(ent->cmd), &len,
>> +				    ent->queue->bufring, issue_flags);
>> +	if (sel.val)
>> +		return sel.val;
>> +	if (!sel.kaddr)
>> +		return -ENOENT;
>> +
>> +	ent->payload_kvec.iov_base = sel.kaddr;
>> +	ent->payload_kvec.iov_len = len;
>> +	ent->ringbuf_buf_id = sel.buf_id;
>> +
>> +	return 0;
>> +}
>> +
>> +static void fuse_uring_clean_up_buffer(struct fuse_ring_ent *ent,
>> +				       unsigned int issue_flags)
>> +	__must_hold(&queue->lock)
>> +{
>> +	struct kvec *kvec = &ent->payload_kvec;
>> +
>> +	lockdep_assert_held(&ent->queue->lock);
>> +
>> +	if (!ent->queue->use_bufring || !kvec->iov_base)
>> +		return;
>> +
>> +	WARN_ON_ONCE(io_uring_kmbuf_recycle(ent->cmd, FUSE_URING_RINGBUF_GROUP,
>> +					    (u64)kvec->iov_base, kvec->iov_len,
>> +					    ent->ringbuf_buf_id, issue_flags));
>> +
>> +	memset(kvec, 0, sizeof(*kvec));
>> +}
>> +
>> +static int fuse_uring_next_req_update_buffer(struct fuse_ring_ent *ent,
>> +					     struct fuse_req *req,
>> +					     unsigned int issue_flags)
>> +{
>> +	bool buffer_selected;
>> +	bool has_payload;
>> +
>> +	if (!ent->queue->use_bufring)
>> +		return 0;
>> +
>> +	ent->headers_iter.data_source = false;
>> +
>> +	buffer_selected = ent->payload_kvec.iov_base != NULL;
>> +	has_payload = fuse_uring_req_has_payload(req);
>> +
>> +	if (has_payload && !buffer_selected)
>> +		return fuse_uring_select_buffer(ent, issue_flags);
>> +
>> +	if (!has_payload && buffer_selected)
>> +		fuse_uring_clean_up_buffer(ent, issue_flags);
>> +
>> +	return 0;
>> +}
>> +
>> +static int fuse_uring_prep_buffer(struct fuse_ring_ent *ent,
>> +				  struct fuse_req *req, unsigned issue_flags)
>> +{
>> +	if (!ent->queue->use_bufring)
>> +		return 0;
>> +
>> +	ent->headers_iter.data_source = false;
>> +
>> +	/* no payload to copy, can skip selecting a buffer */
>> +	if (!fuse_uring_req_has_payload(req))
>> +		return 0;
>> +
>> +	return fuse_uring_select_buffer(ent, issue_flags);
>> +}
>> +
>>  static int fuse_uring_prepare_send(struct fuse_ring_ent *ent,
>>  				   struct fuse_req *req)
>>  {
>> @@ -824,21 +1030,29 @@ static void fuse_uring_add_req_to_ring_ent(struct fuse_ring_ent *ent,
>>  }
>>  
>>  /* Fetch the next fuse request if available */
>> -static struct fuse_req *fuse_uring_ent_assign_req(struct fuse_ring_ent *ent)
>> +static struct fuse_req *fuse_uring_ent_assign_req(struct fuse_ring_ent *ent,
>> +						  unsigned int issue_flags)
>>  	__must_hold(&queue->lock)
>>  {
>>  	struct fuse_req *req;
>>  	struct fuse_ring_queue *queue = ent->queue;
>>  	struct list_head *req_queue = &queue->fuse_req_queue;
>> +	int err;
>>  
>>  	lockdep_assert_held(&queue->lock);
>>  
>>  	/* get and assign the next entry while it is still holding the lock */
>>  	req = list_first_entry_or_null(req_queue, struct fuse_req, list);
>> -	if (req)
>> -		fuse_uring_add_req_to_ring_ent(ent, req);
>> +	if (req) {
>> +		err = fuse_uring_next_req_update_buffer(ent, req, issue_flags);
>> +		if (!err) {
>> +			fuse_uring_add_req_to_ring_ent(ent, req);
>> +			return req;
>> +		}
>> +	}
>>  
>> -	return req;
>> +	fuse_uring_clean_up_buffer(ent, issue_flags);
>> +	return NULL;
>>  }
>>  
>>  /*
>> @@ -878,7 +1092,8 @@ static void fuse_uring_commit(struct fuse_ring_ent *ent, struct fuse_req *req,
>>   * Else, there is no next fuse request and this returns false.
>>   */
>>  static bool fuse_uring_get_next_fuse_req(struct fuse_ring_ent *ent,
>> -					 struct fuse_ring_queue *queue)
>> +					 struct fuse_ring_queue *queue,
>> +					 unsigned int issue_flags)
>>  {
>>  	int err;
>>  	struct fuse_req *req;
>> @@ -886,7 +1101,7 @@ static bool fuse_uring_get_next_fuse_req(struct fuse_ring_ent *ent,
>>  retry:
>>  	spin_lock(&queue->lock);
>>  	fuse_uring_ent_avail(ent, queue);
>> -	req = fuse_uring_ent_assign_req(ent);
>> +	req = fuse_uring_ent_assign_req(ent, issue_flags);
>>  	spin_unlock(&queue->lock);
>>  
>>  	if (req) {
>> @@ -927,6 +1142,39 @@ static void fuse_uring_send(struct fuse_ring_ent *ent, struct io_uring_cmd *cmd,
>>  	io_uring_cmd_done(cmd, ret, issue_flags);
>>  }
>>  
>> +static void fuse_uring_headers_cleanup(struct fuse_ring_ent *ent,
>> +				       unsigned int issue_flags)
>> +{
>> +	if (!ent->queue->use_bufring || !ent->headers_node)
>> +		return;
>> +
>> +	io_uring_fixed_index_put(ent->cmd, ent->headers_node, issue_flags);
>> +	ent->headers_node = NULL;
>> +}
>> +
>> +static int fuse_uring_headers_prep(struct fuse_ring_ent *ent, unsigned int dir,
>> +				   unsigned int issue_flags)
>> +{
>> +	size_t header_size = sizeof(struct fuse_uring_req_header);
>> +	struct io_uring_cmd *cmd = ent->cmd;
>> +	struct io_rsrc_node *node;
>> +	unsigned int offset;
>> +
>> +	if (!ent->queue->use_bufring)
>> +		return 0;
>> +
>> +	offset = ent->fixed_buf_id * header_size;
>> +
>> +	node = io_uring_fixed_index_get(cmd, FUSE_URING_FIXED_HEADERS_OFFSET,
>> +					offset, header_size, dir,
>> +					&ent->headers_iter, issue_flags);
>> +	if (IS_ERR(node))
>> +		return PTR_ERR(node);
>> +
>> +	ent->headers_node = node;
>> +	return 0;
>> +}
>> +
>>  /* FUSE_URING_CMD_COMMIT_AND_FETCH handler */
>>  static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags,
>>  				   struct fuse_conn *fc)
>> @@ -940,6 +1188,7 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags,
>>  	unsigned int qid = READ_ONCE(cmd_req->qid);
>>  	struct fuse_pqueue *fpq;
>>  	struct fuse_req *req;
>> +	bool send;
>>  
>>  	err = -ENOTCONN;
>>  	if (!ring)
>> @@ -990,7 +1239,12 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags,
>>  
>>  	/* without the queue lock, as other locks are taken */
>>  	fuse_uring_prepare_cancel(cmd, issue_flags, ent);
>> -	fuse_uring_commit(ent, req, issue_flags);
>> +
>> +	err = fuse_uring_headers_prep(ent, ITER_SOURCE, issue_flags);
>> +	if (err)
>> +		fuse_uring_req_end(ent, req, err);
>> +	else
>> +		fuse_uring_commit(ent, req, issue_flags);
>>  
>>  	/*
>>  	 * Fetching the next request is absolutely required as queued
>> @@ -998,7 +1252,9 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags,
>>  	 * and fetching is done in one step vs legacy fuse, which has separated
>>  	 * read (fetch request) and write (commit result).
>>  	 */
>> -	if (fuse_uring_get_next_fuse_req(ent, queue))
>> +	send = fuse_uring_get_next_fuse_req(ent, queue, issue_flags);
>> +	fuse_uring_headers_cleanup(ent, issue_flags);
>> +	if (send)
>>  		fuse_uring_send(ent, cmd, 0, issue_flags);
>>  	return 0;


Hello Joanne,

couldn't it call fuse_uring_headers_cleanup() before the
fuse_uring_get_next_fuse_req()? I find it a bit confusing that it firsts
gets the next request and then cleans up the buffer from the previous
request.

As I understand it, the the patch basically adds the feature of 0-byte
payloads. Maybe worth mentioning in the commit message?
I also wonder if it would be worth to document as code comment that
fuse_uring_ent_assign_req / fuse_uring_next_req_update_buffer are
allowed to fail for a buffer upgrade (i.e. 0 to max-payload). At least
the current comment of  "Fetching the next request is absolutely
required" is actually not entirely true anymore.


Thanks,
Bernd