Re: please fix FUSION (Was: [v3.13][v3.14][Regression]kthread:makekthread_create()killable)

[Oleg Nesterov <oleg@redhat.com>]

On 03/22, James Bottomley wrote:
>
> OK, the fix from the SCSI point of view is to make the mpt teardown path
> actually work.  The whole thing looks to be a complete mess because
> there's another place where it will do the wrong thing in
> mptscsih_remove().  You always have to call mpt_detach() otherwise the
> device doesn't get removed from the lists.  In theory this patch fixes
> both bugs in the driver.

Yes, I obviously thought about the same change ;)

But note that mpt_detach() doesn't look correct too. This is almost
off-topic, but still...

At least it needs s/cancel_delayed_work/cancel_delayed_work_sync/ to
sync with mpt_fault_reset_work().

And it is not clear if it is safe to simply destroy ->fw_event_q,
perhaps it can't have the pending works if the caller is teardown
path, and otherwise we rely on mptsas_cleanup_fw_event_q(), I do
not know... but mptsas_cleanup_fw_event_q() needs _sync too I guess,
and obviously mptsas_free_fw_event() should be called unconditionally.

OTOH, mpt_attach() doesn't verify that ->fw_event_q was created
succesfully. And, if mpt_do_ioc_recovery() fails it destroys
->reset_work_q but not ->fw_event_q. Looks like this driver needs
some cleanups.

> --- a/drivers/message/fusion/mptscsih.c
> +++ b/drivers/message/fusion/mptscsih.c
> @@ -1176,10 +1176,14 @@ mptscsih_remove(struct pci_dev *pdev)
>  	MPT_SCSI_HOST		*hd;
>  	int sz1;
>
> +	if (!host)
> +		/* not brought up far enough to do scsi_host_attach() */
                                                   ^^^^^^^^^^^^^^^^
scsi_host_alloc ;)

> +		goto out;
> +

Yes, I think this is correct. mpt_attach() does kzalloc(MPT_ADAPTER) so
we can probably rely on ->sh == NULL if _alloc failed.

Oleg.