Re: [PATCH] Remove sync waiting code from libata
Matthew Wilcox wrote: By using the scsi async probing code, we can remove the 'sync' argument from ata_scsi_scan_host(): Hmmm... How so? @sync is there to keep device numbering stable even when SCSI scan fails due to allocation failure. I don't see how async probing changes that. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove sync waiting code from libata
On Sat, Sep 08, 2007 at 05:14:21PM +0900, Tejun Heo wrote: Matthew Wilcox wrote: By using the scsi async probing code, we can remove the 'sync' argument from ata_scsi_scan_host(): Hmmm... How so? @sync is there to keep device numbering stable even when SCSI scan fails due to allocation failure. I don't see how async probing changes that. async probing also keeps device numbering stable. As long as the device responds within ten seconds (and the current code has half a second as the timeout), it'll get the same number it would have had, even though other hosts have successfully completed their probes during that time. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove sync waiting code from libata
Matthew Wilcox wrote: On Sat, Sep 08, 2007 at 05:14:21PM +0900, Tejun Heo wrote: Matthew Wilcox wrote: By using the scsi async probing code, we can remove the 'sync' argument from ata_scsi_scan_host(): Hmmm... How so? @sync is there to keep device numbering stable even when SCSI scan fails due to allocation failure. I don't see how async probing changes that. async probing also keeps device numbering stable. As long as the device responds within ten seconds (and the current code has half a second as the timeout), it'll get the same number it would have had, even though other hosts have successfully completed their probes during that time. The @sync parameter is to work around GFP_ATOMIC allocation in SCSI scan code. A libata SCSI host can have upto 15 devices with PMP and with multiple devices allocation failure is not so rare, probably because libata SCSI scanning is done back-to-back in rapid succession. The proper fix is probably to make SCSI scanning code not use GFP_ATOMIC (there doesn't seem to be any reason to) but @sync is the bandaid till that happens. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove sync waiting code from libata
Alan Cox wrote: async probing also keeps device numbering stable. As long as the device responds within ten seconds (and the current code has half a second as the timeout), it'll get the same number it would have had, even though other hosts have successfully completed their probes during that time. The usual probe time for a device which is spun down is nearer 30 seconds. libata probing is done differently tho. It goes like... 1. libata EH probes all hardware. Things can take quite long here but SCSI isn't really involved. 2. After libata EH is finished, SCSI host scan is invoked. It doesn't do much. When SCSI probing command reaches libata-scsi, it just fakes the replies from the information gathered in #1 - No actual ATA reset or command is issued. If there are multiple devices on a host (ATA port), this rapid probing makes SCSI host scan code fail GFP_ATOMIC allocation from time to time. This is why the @sync hack was used. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Remove sync waiting code from libata
By using the scsi async probing code, we can remove the 'sync' argument
from ata_scsi_scan_host():
diff -u b/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
--- b/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6362,10 +6362,15 @@
}
}
+/*
+ * Give ourselves ten seconds to find everything.
+ * This should be more than enough time
+ */
int ata_scsi_scan_finished(struct Scsi_Host *shost, unsigned long time)
{
- ata_scsi_scan_host(ata_shost_to_port(shost), 1);
- return 1;
+ if (ata_scsi_scan_host(ata_shost_to_port(shost)))
+ return 1;
+ return time (10 * HZ);
}
/**
diff -u b/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
--- b/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2954,17 +2954,15 @@
return rc;
}
-void ata_scsi_scan_host(struct ata_port *ap, int sync)
+/* Returns 1 if we are done scanning, 0 if we've scheduled async scanning */
+int ata_scsi_scan_host(struct ata_port *ap)
{
- int tries = 5;
- struct ata_device *last_failed_dev = NULL;
struct ata_device *dev;
unsigned int i;
if (ap-flags ATA_FLAG_DISABLED)
- return;
+ return 1;
- repeat:
for (i = 0; i ATA_MAX_DEVICES; i++) {
struct scsi_device *sdev;
@@ -2990,34 +2988,11 @@
break;
}
if (i == ATA_MAX_DEVICES)
- return;
-
- /* we're missing some SCSI devices */
- if (sync) {
- /* If caller requested synchrnous scan we've made
-* any progress, sleep briefly and repeat.
-*/
- if (dev != last_failed_dev) {
- msleep(100);
- last_failed_dev = dev;
- goto repeat;
- }
-
- /* We might be failing to detect boot device, give it
-* a few more chances.
-*/
- if (--tries) {
- msleep(100);
- goto repeat;
- }
-
- ata_port_printk(ap, KERN_ERR, WARNING: synchronous SCSI scan
- failed without making any progress,\n
- switching to async\n);
- }
+ return 1;
queue_delayed_work(ata_aux_wq, ap-hotplug_task,
round_jiffies_relative(HZ));
+ return 0;
}
/**
@@ -3144,7 +3119,7 @@
}
/* scan for new ones */
- ata_scsi_scan_host(ap, 0);
+ ata_scsi_scan_host(ap);
DPRINTK(EXIT\n);
}
only in patch2:
unchanged:
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -112,7 +112,7 @@ static inline int ata_acpi_on_devcfg(struct ata_device
*adev) { return 0; }
/* libata-scsi.c */
extern int ata_scsi_add_hosts(struct ata_host *host,
struct scsi_host_template *sht);
-extern void ata_scsi_scan_host(struct ata_port *ap, int sync);
+extern int ata_scsi_scan_host(struct ata_port *ap);
extern int ata_scsi_offline_dev(struct ata_device *dev);
extern void ata_scsi_hotplug(struct work_struct *work);
extern unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf,
--
Intel are signing my paycheques ... these opinions are still mine
Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
