> -----Original Message----- > From: Bart Van Assche [mailto:Bart.VanAssche@xxxxxxxxxxx] > Sent: Tuesday, June 13, 2017 8:38 AM > To: linux-kernel@xxxxxxxxxxxxxxx; KY Srinivasan <kys@xxxxxxxxxxxxx>; > martin.petersen@xxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; Stephen Hemminger > <sthemmin@xxxxxxxxxxxxx>; Long Li <longli@xxxxxxxxxxxxx>; > jejb@xxxxxxxxxxxxxxxxxx; Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> > Cc: jthumshirn@xxxxxxx; Long Li <longli@xxxxxxxxxxxxx> > Subject: Re: [Possible Phish Fraud][PATCH] storvsc: use default I/O timeout > handler for FC devices > > On Mon, 2017-06-12 at 17:23 -0700, Long Li wrote: > > From: Long Li <longli@xxxxxxxxxxxxx> > > > > FC disks are usually setup in a multipath system, and they don't want > > to unconditionaly reset I/O on timeout. I/O timeout is detected by > > multipath as a good time to failover and recover. > > > > Signed-off-by: Long Li <longli@xxxxxxxxxxxxx> > > --- > > drivers/scsi/storvsc_drv.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > > index 8d955db..d60b5ea 100644 > > --- a/drivers/scsi/storvsc_drv.c > > +++ b/drivers/scsi/storvsc_drv.c > > @@ -486,6 +486,7 @@ struct hv_host_device { > > unsigned int port; > > unsigned char path; > > unsigned char target; > > + bool is_fc; > > }; > > > > struct storvsc_scan_work { > > @@ -1495,6 +1496,11 @@ static int storvsc_host_reset_handler(struct > scsi_cmnd *scmnd) > > */ > > static enum blk_eh_timer_return storvsc_eh_timed_out(struct scsi_cmnd > > *scmnd) { > > + struct hv_host_device *host_dev = shost_priv(scmnd->device->host); > > + > > + if (host_dev->is_fc) > > + return BLK_EH_NOT_HANDLED; > > + > > return BLK_EH_RESET_TIMER; > > } > > > > @@ -1738,6 +1744,7 @@ static int storvsc_probe(struct hv_device > > *device, > > > > host_dev->port = host->host_no; > > host_dev->dev = device; > > + host_dev->is_fc = is_fc; > > > > > > stor_device = kzalloc(sizeof(struct storvsc_device), GFP_KERNEL); > > Hello Long, > > As far as I know there is no other SCSI driver nor block driver in the Linux kernel > tree that returns BLK_EH_RESET_TIMER unconditionally. Would a valid > alternative fix be to remove storvsc_eh_timed_out() entirely? If not, what > would break if that function would be removed entirely? Storvsc handles virtualized devices for Hyper-V and Azure. The host decides on how to handle I/O timeout, and makes decision based on how long I/O has been in flight. This is why we use BLK_EH_RESET_TIMER and leave the I/O timeout handling to the host for most cases. > > Additionally, for FC, shouldn't that timeout handler handle the "port blocked" > state? Shouldn't fc_eh_timed_out() be used for FC instead of just returning > BLK_EH_NOT_HANDLED? I agree with you. Storvsc didn't expose rport state. But this has changed with a recent commit daf0cd445a218314f9461d67d4f2b9c24cdd534b. I'll send a V2 to address this. Thank you Long > > Thanks, > > Bart.
