All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: io-uring@vger.kernel.org, Pavel Begunkov <asml.silence@gmail.com>,
	Caleb Sander Mateos <csander@purestorage.com>
Subject: Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
Date: Tue, 19 Aug 2025 22:55:21 +0800	[thread overview]
Message-ID: <aKSQWUXW2k97SQoQ@fedora> (raw)
In-Reply-To: <06bd405d-c5dd-4f20-af90-775278686f5c@kernel.dk>

On Tue, Aug 19, 2025 at 08:43:58AM -0600, Jens Axboe wrote:
> On 8/19/25 8:40 AM, Ming Lei wrote:
> > On Tue, Aug 19, 2025 at 08:31:32AM -0600, Jens Axboe wrote:
> >> On 8/19/25 8:28 AM, Ming Lei wrote:
> >>> On Tue, Aug 19, 2025 at 08:01:18AM -0600, Jens Axboe wrote:
> >>>> On 8/19/25 5:19 AM, Ming Lei wrote:
> >>>>>>> @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> >>>>>>>  	}
> >>>>>>>  
> >>>>>>>  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> >>>>>>> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> >>>>>>> +		if (ret >= 0)
> >>>>>>> +			return IOU_ISSUE_SKIP_COMPLETE;
> >>>>>>> +		io_kbuf_recycle(req, issue_flags);
> >>>>>>> +	}
> >>>>>>>  	if (ret == -EAGAIN) {
> >>>>>>>  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
> >>>>>>>  		return ret;
> >>>>>>
> >>>>>> Missing recycle for -EAGAIN?
> >>>>>
> >>>>> io_kbuf_recycle() is done above if `ret < 0`
> >>>>
> >>>> Inside the multishot case. I don't see anywhere where it's forbidden to
> >>>> use IOSQE_BUFFER_SELECT without having multishot set? Either that needs
> >>>
> >>> REQ_F_BUFFER_SELECT is supposed to be allowed for IORING_URING_CMD_MULTISHOT
> >>> only, and it is checked in io_uring_cmd_prep().
> >>>
> >>>> to be explicit for now, or the recycling should happen generically.
> >>>> Probably the former I would suspect.
> >>>
> >>> Yes, the former is exactly what the patch is doing.
> >>
> >> Is it? Because looking at v2, you check if IORING_URING_CMD_FIXED is
> >> set, and you fail for that case if REQ_F_BUFFER_SELECT is set. Then you
> >> have a IORING_URING_CMD_MULTISHOT where the opposite is true, which
> >> obviously makes sense.
> >>
> >> But no checks if neither is set?
> > 
> > Indeed, thanks for the catch, and the REQ_F_BUFFER_SELECT check in IORING_URING_CMD_FIXED
> > branch can be moved to the branch of !IORING_URING_CMD_MULTISHOT.
> > 
> >>
> >> You could add that in io_uring_cmd_select_buffer(), eg fail if
> >> IORING_URING_CMD_MULTISHOT isn't set. Which if done, then the prep side
> >> checking could probably just go away.
> > 
> > Looks this way is good too.
> 
> Want to spin a v3 for this?

Yeah, will send V3 out after test is done.

Thanks,
Ming


      reply	other threads:[~2025-08-19 14:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-10  2:50 [PATCH] io_uring: uring_cmd: add multishot support without poll Ming Lei
2025-08-18  2:16 ` Ming Lei
2025-08-18 15:31 ` Jens Axboe
2025-08-19 11:19   ` Ming Lei
2025-08-19 14:01     ` Jens Axboe
2025-08-19 14:28       ` Ming Lei
2025-08-19 14:31         ` Jens Axboe
2025-08-19 14:40           ` Ming Lei
2025-08-19 14:43             ` Jens Axboe
2025-08-19 14:55               ` Ming Lei [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aKSQWUXW2k97SQoQ@fedora \
    --to=ming.lei@redhat.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=csander@purestorage.com \
    --cc=io-uring@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.