Mark Lord wrote: > Tejun, > > This Oops (2.6.23.13) looks very much like the same bug > you fixed recently for me in 2.6.24. > > The bug was with sata_qstor and other drivers, in that > devres/libata were freeing the I/O resources before invoking > the LLD's host/port_stop routines .. which still need the I/O. > > Did that patch get backported to 2.6.23 yet ? No, it's not but yeah it probably should have. Tomasz, can you please test whether the attached patch fixes the problem? -- tejun
Index: tree0/drivers/ata/libata-core.c =================================================================== --- tree0.orig/drivers/ata/libata-core.c +++ tree0/drivers/ata/libata-core.c @@ -6121,19 +6121,6 @@ static void ata_host_release(struct devi if (!ap) continue; - if ((host->flags & ATA_HOST_STARTED) && ap->ops->port_stop) - ap->ops->port_stop(ap); - } - - if ((host->flags & ATA_HOST_STARTED) && host->ops->host_stop) - host->ops->host_stop(host); - - for (i = 0; i < host->n_ports; i++) { - struct ata_port *ap = host->ports[i]; - - if (!ap) - continue; - if (ap->scsi_host) scsi_host_put(ap->scsi_host); @@ -6258,6 +6245,24 @@ struct ata_host *ata_host_alloc_pinfo(st return host; } +static void ata_host_stop(struct device *gendev, void *res) +{ + struct ata_host *host = dev_get_drvdata(gendev); + int i; + + WARN_ON(!(host->flags & ATA_HOST_STARTED)); + + for (i = 0; i < host->n_ports; i++) { + struct ata_port *ap = host->ports[i]; + + if (ap->ops->port_stop) + ap->ops->port_stop(ap); + } + + if (host->ops->host_stop) + host->ops->host_stop(host); +} + /** * ata_host_start - start and freeze ports of an ATA host * @host: ATA host to start ports for @@ -6276,6 +6281,8 @@ struct ata_host *ata_host_alloc_pinfo(st */ int ata_host_start(struct ata_host *host) { + int have_stop = 0; + void *start_dr = NULL; int i, rc; if (host->flags & ATA_HOST_STARTED) @@ -6287,6 +6294,22 @@ int ata_host_start(struct ata_host *host if (!host->ops && !ata_port_is_dummy(ap)) host->ops = ap->ops; + if (ap->ops->port_stop) + have_stop = 1; + } + + if (host->ops->host_stop) + have_stop = 1; + + if (have_stop) { + start_dr = devres_alloc(ata_host_stop, 0, GFP_KERNEL); + if (!start_dr) + return -ENOMEM; + } + + for (i = 0; i < host->n_ports; i++) { + struct ata_port *ap = host->ports[i]; + if (ap->ops->port_start) { rc = ap->ops->port_start(ap); if (rc) { @@ -6299,6 +6322,8 @@ int ata_host_start(struct ata_host *host ata_eh_freeze_port(ap); } + if (start_dr) + devres_add(host->dev, start_dr); host->flags |= ATA_HOST_STARTED; return 0; @@ -6309,6 +6334,7 @@ int ata_host_start(struct ata_host *host if (ap->ops->port_stop) ap->ops->port_stop(ap); } + devres_free(start_dr); return rc; }