On Mon, May 12, 2025 at 8:05 PM Ming Lei <ming.lei@xxxxxxxxxx> wrote: > > On Mon, May 12, 2025 at 01:25:35PM -0700, Caleb Sander Mateos wrote: > > On Fri, May 9, 2025 at 8:06 AM Ming Lei <ming.lei@xxxxxxxxxx> wrote: > > > > > > `ublk_queue` is read only for io buffer register/unregister command. Both > > > `ublk_io` and block layer request are read-only for IO buffer register/ > > > unregister command. > > > > > > So the two command can be issued from other task contexts. > > > > > > Not same with other three ublk commands, these two are for handling target > > > IO only, we shouldn't limit their issue task context, otherwise it becomes > > > hard for ublk server(backend) to use zero copy feature. > > > > > > Reported-by: Uday Shankar <ushankar@xxxxxxxxxxxxxxx> > > > Closes: https://lore.kernel.org/linux-block/20250410-ublk_task_per_io-v3-2-b811e8f4554a@xxxxxxxxxxxxxxx/ > > > > I don't agree that this change obviates the need for per-io tasks. > > Being able to perform zero-copy buffer registration on other threads > > You may misunderstand the concern, it isn't for solving load balancing, > it is just for making zero copy easier to use. > > Not like other uring_cmd(FETCH, COMMIT_AND_FETCH), register io buffer is > for target IO handling, which shouldn't be limited in the ubq_daemon > context, Uday did mention this point in above link. I think we are in agreement, I just interpreted the "Closes" tag differently. "Closes" to me suggests this commit addresses all the concerns motivating Uday's patch (making that patch unnecessary). Uday's comment in the commit message that "The problem gets even worse with zero copy, as more inter-thread communication would be required to have the buffer register/unregister calls to come from the correct thread" seemed somewhat tangential to me. The primary concern is about having to handle incoming ublk I/Os on a single thread or pay the cache line contention cost of sending them between threads, which still applies even if the zero-copy buffer (un)register can be performed on a different thread. Maybe a "Link" tag would be more appropriate? > > > can't help with spreading the load if the ublk server isn't using > > zero-copy in the first place. And sending I/Os between ublk server > > threads adds cross-CPU synchronization overhead (as Uday points out in > > the commit message for his change). Distributing I/Os among the ublk > > server threads at the point where the blk-mq request is queued seems > > like a natural place to do load balancing, as the request is already > > being sent between CPUs there. > > I do agree load balancing should be addressed, together with relaxing > existing ublk server context limitation. Agreed, I think both Uday's and your changes make sense. Best, Caleb
