On 2021-02-11 7:13 a.m., Kashyap Desai wrote:
Kashyap, There is another issue here, namely iodepth > host_max_queue (64 > 32) and in my setup that is not handled well. So there is a problem with scsi_debug *** or the mid-level in handling this case. If you have modified the sd driver to call blk_poll() then perhaps you could try the above test again with a reduced iodepth.Doug - I debug the issue and found the fix. We need to remove below code to handle this new defer type for mq poll. Earlier I did not used " REQ_HIPRI" to handle mq poll, but now you are managing it through REQ_HIPRI check, it is safe and required to have below patch. diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index b9ec3a47fbf1..c50de49a2c2f 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -5437,13 +5437,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, sd_dp = sqcp->sd_dp; spin_unlock_irqrestore(&sqp->qc_lock, iflags); - /* Do not complete IO from default completion path. - * Let it to be on queue. - * Completion should happen from mq_poll interface. - */ - if ((sqp - sdebug_q_arr) >= (submit_queues - poll_queues)) - return 0; - if (!sd_dp) { sd_dp = kzalloc(sizeof(*sd_dp), GFP_ATOMIC); if (!sd_dp) { On top of your v3 patch + above fix, I am able to run IO and there are no issues. There is no issue with higher QD as well, so we are good to go. Tested-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx> So how shall we progress now ? Shall I ask Martin to pick all the patches from " [PATCH v3 0/4] io_uring iopoll in scsi layer" since this patch has dependency on my series.
Kashyap, Good catch! You could roll all the scsi_debug stuff into one patch and put both our sign-offs on it, or present it as two patches: - first one: your original plus this fix - second one: mine with your tested-by on it Since this is new functionality for the scsi sub-system, it is unlikely that all the corner cases have been visited. However, with what we have in place for the next cycle, others have something reasonably solid to build on. Doug Gilbert
*** the scsi_debug should either yield a TASK SET FULL status or return SCSI_MLQUEUE_HOST_BUSY from queuecommand() when it has run out of slots.[seqprecon] filename=/dev/sdd [seqprecon] filename=/dev/sde [seqprecon] filename=/dev/sdf I will ask Martin to pick all the patches from "[v3,1/4] add io_uring with IOPOLL support in the scsi layer" except scsi_debug. We can work on scsi_debug and send standalone patch. Kashyap
