Re: [PATCHv5 09/11] ublk: zc register/unregister bvec

[Ming Lei <ming.lei@redhat.com>]

On Tue, Feb 25, 2025 at 09:52:09AM -0700, Keith Busch wrote:
> On Tue, Feb 25, 2025 at 04:42:59PM +0000, Pavel Begunkov wrote:
> > On 2/25/25 16:27, Keith Busch wrote:
> > > On Tue, Feb 25, 2025 at 04:19:37PM +0000, Pavel Begunkov wrote:
> > > > On 2/24/25 21:31, Keith Busch wrote:
> > > > > From: Keith Busch <kbusch@kernel.org>
> > > > > 
> > > > > Provide new operations for the user to request mapping an active request
> > > > > to an io uring instance's buf_table. The user has to provide the index
> > > > > it wants to install the buffer.
> > > > 
> > > > Do we ever fail requests here? I don't see any result propagation.
> > > > E.g. what if the ublk server fail, either being killed or just an
> > > > io_uring request using the buffer failed? Looking at
> > > > __ublk_complete_rq(), shouldn't someone set struct ublk_io::res?
> > > 
> > > If the ublk server is killed, the ublk driver timeout handler will abort
> > > all incomplete requests.
> > > 
> > > If a backend request using this buffer fails, for example -EFAULT, then
> > > the ublk server notifies the ublk driver frontend with that status in a
> > > COMMIT_AND_FETCH command, and the ublk driver completes that frontend
> > > request with an appropriate error status.
> > 
> > I see. IIUC, the API assumes that in normal circumstances you
> > first unregister the buffer, and then issue another command like
> > COMMIT_AND_FETCH to finally complete the ublk request. Is that it?
> 
> Yes, that's the expected good sequence. It's okay if user space does it

That is exactly what the ublk ktests patch loop/zc is doing.

> the other around, too: commit first, then unregister. The registration
> holds a reference on the ublk request, preventing it from completing.

It depends on UBLK_IO_COMMIT_AND_FETCH_REQ is only done once, and luckily
it works in this way from beginning, otherwise the request still may be freed
before unregister command is done.

> The backend urin gthat registered the bvec can also be a different uring
> instance than the frontend that notifies the of the commit-and-fetch. In
> such a setup, the commit and unregister sequence could happen
> concurrently, and that's also okay.

Yes, ublk io commands are always run in the queue context, but
unregister from io-wq still brings some extra latency for io.




Thanks,
Ming