On Tue, 2023-04-11 at 15:09 +0200, Tomas Henzl wrote:
> On 4/8/23 21:18, Jerry Snitselaar wrote:
> > We've had some people trying to track a problem for months
> > revolving
> > around a system hanging at shutdown, and last thing they see being
> > a
> > message from mpt3sas about a reset. They quickly bisected down to
> > the
> > commit below, and reverted it made the problem go away for the
> > customer.
> >
> > b424eaa1b51c ("scsi: mpt3sas: Transition IOC to Ready state during
> > shutdown")
> >
> > I got asked to look at something since I recently at another issue
> > that involved mpt3sas at shutdown, so I was looking through the
> > history, saw this commit being mentined. Looking at it, I'm not
> > sure
> > why it is doing what is doing.
> >
> > It says it is to perform a soft reset, but that was already
> > happening before this commit via:
> >
> > scsih_shutdown -> mpt3sas_base_detach ->
> > mpt3sas_base_free_resources -> _base_make_ioc_ready(ioc,
> > SOFT_RESET);
> >
> > The original submission [1] had the following commit message:
> >
> > "During shutdown just move the IOC state to Ready state
> > by issuing MUR. No need to free any IOC memory pools."
> >
> > But is now skipping more than not freeing the memory pools. It no
> > longer frees memory that was kalloc'd, it doesn't unmap something
> > that
> > was iomapped, it no longer cleans up the fault reset workqueue, and
> > no
> > longer calls the pci cleanup code. It also no longer does the
> > things
> > it moved to scsih_shutdown under the pci access mutex, nor uses the
> > if
> > condition that was in mpt3sas_base_free_resources.
> >
> > [1]
> > https://lore.kernel.org/r/20210705145951.32258-1-sreekanth.reddy@xxxxxxxxxxxx
> >
> >
> > Am I missing something, and what the commit does here is really
> > okay?
>
> When a driver's shutdown method is called it may be still processing
> in
> parallel I/Os (that may also happen any time later) so not releasing
> the
> resources the driver has allocated is correct. A next step is then a
> power off or a boot of a new kernel anyway.
> A driver should stop DMA transfers and IRQ's, silence itself and when
> needed inform the attached hw to flush memory or whatever else.
>
Should it clean up dma mappings then? All of that memory pool stuff is
still mapped.
Thanks for the info.
Regards,
Jerry
> (The fix I've posted for the DMA issue in shutdown has this subject
> 'mpt3sas: fix an issue when driver's being removed')
>
> Regards,
> Tomas
>
> >
> > Regards,
> > Jerry
> >
>
