Re: [PATCH] Correct some mistakes in drivers using the scsi hotplug model

James Bottomley
2007-07-31T14:42:27Z

On Tue, 2007-07-31 at 06:55 -0600, Matthew Wilcox wrote:
> A few drivers are doing things like releasing IRQs before calling
> scsi_remove_host(), which can lead to some ugly error messages.
> Many more drivers are simply forgetting to call scsi_host_put() after
> they've finished tearing down their driver structures.
>
> Signed-off-by: Matthew Wilcox <matthew@wil.cx>
>
> diff --git a/drivers/scsi/NCR_D700.c b/drivers/scsi/NCR_D700.c
> index 3a80897..6c2d4bb 100644
> --- a/drivers/scsi/NCR_D700.c
> +++ b/drivers/scsi/NCR_D700.c
> @@ -357,6 +357,7 @@ NCR_D700_remove_one(struct Scsi_Host *host)
> kfree((struct NCR_700_Host_Parameters *)host->hostdata[0]);
> free_irq(host->irq, host);
> release_region(host->base, 64);
> + scsi_host_put(host);
> }

Rather than changing this in every 53c700 based driver, shouldn't it
just be added to NCR_700_release()?

> static int __devexit
> diff --git a/drivers/scsi/NCR_Q720.c b/drivers/scsi/NCR_Q720.c
> index a8bbdc2..6f03574 100644
> --- a/drivers/scsi/NCR_Q720.c
> +++ b/drivers/scsi/NCR_Q720.c
> @@ -323,6 +323,7 @@ NCR_Q720_remove_one(struct Scsi_Host *host)
> {
> scsi_remove_host(host);
> ncr53c8xx_release(host);
> + scsi_host_put(host);
> }

Ditto for this (since zalon also has the problem).

> static int __exit
> diff --git a/drivers/scsi/a4000t.c b/drivers/scsi/a4000t.c
> index 0c758d1..8211225 100644
> --- a/drivers/scsi/a4000t.c
> +++ b/drivers/scsi/a4000t.c
> @@ -105,6 +105,7 @@ static __devexit int a4000t_device_remove(struct device *dev)
> kfree(hostdata);
> free_irq(host->irq, host);
> release_mem_region(A4000T_SCSI_ADDR, 0x1000);
> + scsi_host_put(host);
>
> return 0;
> }
> diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
> index 85f2394..27acef4 100644
> --- a/drivers/scsi/aha152x.c
> +++ b/drivers/scsi/aha152x.c
> @@ -907,6 +907,7 @@ void aha152x_release(struct Scsi_Host *shpnt)
> if(!shpnt)
> return;
>
> + scsi_remove_host(shpnt);
> if (shpnt->irq)
> free_irq(shpnt->irq, shpnt);

and this one needs to go via Boaz ... he's currently rewriting this
driver.

James