On Tue, Jul 12, 2022 at 10:33:34AM +0800, Ming Lei wrote: > Hi Gabriel, > > On Mon, Jul 11, 2022 at 04:06:04PM -0400, Gabriel Krisman Bertazi wrote: > > Ming Lei <ming.lei@xxxxxxxxxx> writes: > > > > > Add UBLK_IO_REFETCH_REQ command to fetch the incoming io request in > > > ubq daemon context, so we can avoid to call task_work_add(), then > > > it is fine to build ublk driver as module. > > > > > > In this way, iops is affected a bit, but just by ~5% on ublk/null, > > > given io_uring provides pretty good batching issuing & completing. > > > > > > One thing to be careful is race between ->queue_rq() and handling > > > abort, which is avoided by quiescing queue when aborting queue. > > > Except for that, handling abort becomes much easier with > > > UBLK_IO_REFETCH_REQ since aborting handler is strictly exclusive with > > > anything done in ubq daemon kernel context. > > > > Hi Ming, > > > > FWIW, I'm not very fond this change. It adds complexity to the kernel > > driver and to the userspace server implementation, who now have to deal > > IMO, this way just adds dozens line of code, no much complexity. The only > complexity in ublk driver should be in aborting code, which is actually > originated from concurrent aborting work and running task work which may be > run after task is exiting. But any storage driver's aborting/error > handling code is complicated. > > Using REFETCH_REQ actually becomes much easier for handling abort which is > run exclusively with any code running in ubq daemon context, but with > performance cost. > > > with different interface semantics just because the driver was built-in > > or built as a module. I don't think the tristate support warrants such > > complexity. I was hoping we might get away with exporting that symbol > > or adding a built-in ubd-specific wrapper that can be exported and > > invokes task_work_add. > > If task_work_add can be exported, that would be very great. Another choice is to use io_uring_cmd_complete_in_task which is actually exported, now we can build ublk_drv as module by using io_uring_cmd_complete_in_task without needing one new command. Thanks, Ming
