On 2021-04-14 2:19 p.m., John Garry wrote:

On 14/04/2021 18:03, Douglas Gilbert wrote:

quoted

On 2021-04-14 9:59 a.m., Kashyap Desai wrote:

quoted

quoted

quoted

I tried both - 5.12.0-rc1 and 5.11.0-rc2+ and there is a same

behavior.

quoted

quoted

Let me also check  megaraid_sas and see if anything generic or this is
a special case of scsi_debug.

As I mentioned, it could be one generic issue wrt. SCHED_RESTART.
shared tags might have to restart all hctx since all share same tags.

Ming - I tried many combination on MR shared host tag driver but there is
no single instance of IO hang.
I will keep trying, but when I look at scsi_debug driver code I found
below odd settings in scsi_debug driver.
can_queue of adapter is set to 128 but queue_depth of sdev is set to 255.

If I apply below patch, scsi_debug driver's hang is also resolved. Ideally
sdev->queue depth cannot exceed shost->can_queue.
Not sure why cmd_per_lun is 255 in scsi_debug driver which can easily
exceed can_queue.  I will simulate something similar in MR driver and see
how it behaves w.r.t IO hang issue.

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 70165be10f00..dded762540ee 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c

@@ -218,7 +218,7 @@ static const char *sdebug_version_date = "20200710";

   */
  #define SDEBUG_CANQUEUE_WORDS  3       /* a WORD is bits in a long */
  #define SDEBUG_CANQUEUE  (SDEBUG_CANQUEUE_WORDS * BITS_PER_LONG)

So SDEBUG_CANQUEUE is 3*64 = 192 and is a hard limit (it is used to
dimension an array). Should it be upped to 4, say? [That will slow things
down a bit if that is an issue.]

sdev_store_queue_depth() enforces that the sdev queue depth cannot exceed 
can_queue.

That is only invoked by the user doing:
     echo [qdepth] > /sys/class/scsi_device/[hctl]/device/queue_depth

For scsi_debug that sysfs store leads to sdebug_change_qdepth() being
invoked. That function currently contains this hack:

         /* allow to exceed max host qc_arr elements for testing */
         if (qdepth > SDEBUG_CANQUEUE + 10)
                 qdepth = SDEBUG_CANQUEUE + 10;
         scsi_change_queue_depth(sdev, qdepth);

Maybe that hack should be replaced with:
	if (qdepth <= SDEBUG_CANQUEUE)
		scsi_change_queue_depth(sdev, qdepth);
	else
		<print warning>
	return sdev->queue_depth;

That hack has been useful in my testing (e.g. currently I'm tracking down
an oops caused by it in my sg driver rewrite) but it may confuse others.
There are more clear cut ways to make the scsi_debug driver inject errors.

Doug Gilbert

I don't know why this is not also enforced in scsi_alloc_sdev(), or even when 
registering the shost (for cmd_per_lun)

quoted

quoted

-#define DEF_CMD_PER_LUN  255
+#define DEF_CMD_PER_LUN  SDEBUG_CANQUEUE

  /* UA - Unit Attention; SA - Service Action; SSU - Start Stop Unit */
  #define F_D_IN                 1       /* Data-in command (e.g. READ) */

@@ -7558,6 +7558,7 @@ static int sdebug_driver_probe(struct device *dev)

         sdbg_host = to_sdebug_host(dev);

         sdebug_driver_template.can_queue = sdebug_max_queue;
+       sdebug_driver_template.cmd_per_lun = sdebug_max_queue;

I'll  push out a patch shortly.

Doug Gilbert

quoted

         if (!sdebug_clustering)
                 sdebug_driver_template.dma_boundary = PAGE_SIZE - 1;

quoted

Thanks,
Ming

.