Hello Richard, I know you just sent an email stating that you will be reworking the logic but I've a question. I haven't looked at the call-flow in detail but consider the following case: 1) IO is streaming to LUN 'x'. 2) LUN 'x' was unmapped from a target. Trust me there are arrays that will let you do that ;). 3) Initiator issued a 'rescan'. 4) LUN 'x' is missing from the response. 4.1) Now we can't just yank that missing LUN 'x'. This is why: A) Outstanding IO's will need some time to abort. Example: FC EDTOV etc might be ~30s. B) SCSI-mid should know that it should NOT retry the aborted IO because the device is scheduled for deletion. Otherwise it will be easy to induce an abort storm. Will __scsi_remove_device take care of all the scsi_device state transition? Chetan Loke > -----Original Message----- > From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- > owner@xxxxxxxxxxxxxxx] On Behalf Of Konrad Rzeszutek Wilk > Sent: August 16, 2010 9:45 AM > To: realrichardsharpe@xxxxxxxxx > Cc: linux-scsi@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 1/1] Remove LUNs that no longer exist when we scan > a target with REPORT LUNS. > > On Sunday 15 August 2010 20:08:33 realrichardsharpe@xxxxxxxxx wrote: > > From: Richard Sharpe <realrichardsharpe@xxxxxxxxx> > > > > If the target returns logical_unit_not_supported when we send REPORT > LUNS > > it means that it supports REPORT LUNS but there really are no LUNs > there. > > Delete LUN 0 in that case. > > > > Also, when parsing the LUNs reported, remove any LUNs that used to > exist > > in the gaps, and remove LUNs beyond the end of those reported. They > no > > longer exist. > > > > Also don't scan a target where the ID is too large or the channel is > > too large. > > > > Tested by adding four LUNs with scst_local and then deleting them in > > various combinations, including deleting from LUN 0, deleting from > last > > LUN and deleting in the middle out. > > What happens if you stack the device? Say you got multipath on top of > this or > device-mapper? Does this work properly ? For multipath I would expect > it to > work OK, and multipath device would queue the I/O until a LUN with the > same > SCSI_ID would reappear. > > > > Signed-off-by: Richard Sharpe <realrichardsharpe@xxxxxxxxx> > > --- > > drivers/scsi/scsi_scan.c | 65 > > +++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 62 > > insertions(+), 3 deletions(-) > > > > diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c > > index 1c027a9..d0f5d30 100644 > > --- a/drivers/scsi/scsi_scan.c > > +++ b/drivers/scsi/scsi_scan.c > > @@ -1309,7 +1309,7 @@ static int scsi_report_lun_scan(struct > scsi_target > > *starget, int bflags, char devname[64]; > > unsigned char scsi_cmd[MAX_COMMAND_SIZE]; > > unsigned int length; > > - unsigned int lun; > > + unsigned int lun, old_lun = 0; > > unsigned int num_luns; > > unsigned int retries; > > int result; > > @@ -1410,6 +1410,22 @@ static int scsi_report_lun_scan(struct > scsi_target > > *starget, int bflags, if (result == 0) > > break; > > else if (scsi_sense_valid(&sshdr)) { > > + /* > > + * This is awful. We should use a named constant! > > + * If the target returns logical_unit_not_supported > > + * then there really are zero LUNs there, so delete > > + * LUN 0 if it exists. > > + */ > > + if (sshdr.asc == 0x25) { > > + struct scsi_device *sdev = NULL; > > + sdev = scsi_device_lookup_by_target(starget, > 0); > > + if (sdev) { > > + scsi_device_put(sdev); > > + __scsi_remove_device(sdev); > > + SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO > > + "Removing LUN 0\n")); > > + } > > + } > > if (sshdr.sense_key != UNIT_ATTENTION) > > break; > > } > > @@ -1473,6 +1489,27 @@ static int scsi_report_lun_scan(struct > scsi_target > > *starget, int bflags, } else { > > int res; > > > > + /* > > + * First, check for and remove any missing LUNs, as > > + * they have been removed on the target. > > + */ > > + if (lun > old_lun + 1) { > > + unsigned int i; > > + for (i = old_lun + 1; i < lun; i++) { > > + struct scsi_device *sdev; > > + sdev = scsi_device_lookup_by_target( > > + starget, i); > > + if (sdev) { > > + scsi_device_put(sdev); > > + __scsi_remove_device(sdev); > > + SCSI_LOG_SCAN_BUS(3, > > + printk(KERN_INFO > > + "Removing LUN %d\n", > > + i)); > > + } > > + } > > + } > > + > > res = scsi_probe_and_add_lun(starget, > > lun, NULL, NULL, rescan, NULL); > > if (res == SCSI_SCAN_NO_RESPONSE) { > > @@ -1485,6 +1522,26 @@ static int scsi_report_lun_scan(struct > scsi_target > > *starget, int bflags, " aborted\n", lun); > > break; > > } > > + > > + old_lun = lun; > > + } > > + } > > + > > + /* > > + * Remove any device that no longer exist. Does not work for LUN > 0. > > + * See above. > > + */ > > + if (lun < shost->max_lun) { > > + int i = 0; > > + for (i = lun + 1; i < shost->max_lun; i++) { > > + struct scsi_device *sdev; > > + sdev = scsi_device_lookup_by_target(starget, i); > > + if (sdev) { > > + scsi_device_put(sdev); > > + __scsi_remove_device(sdev); > > + SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO > > + "Removed LUN %d\n", i)); > > + } > > } > > } > > > > @@ -1565,9 +1622,11 @@ static void __scsi_scan_target(struct device > > *parent, unsigned int channel, int res; > > struct scsi_target *starget; > > > > - if (shost->this_id == id) > > + if (shost->this_id == id || > > + id > shost->max_id || > > + channel > shost->max_channel) > > /* > > - * Don't scan the host adapter > > + * Don't scan the host adapter or dis-allowed ones. > > */ > > return; > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" > in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
