On Sat, Apr 08, 2023 at 01:59:21PM -0700, Jerry Snitselaar wrote:
> On Sat, Apr 08, 2023 at 12:18:29PM -0700, 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")
> >
>
> That should be (grabbed the wrong commit id):
>
> fae21608c31c ("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?
> >
It looks like this deals with the fault reset workqueue no longer being cleaned up:
0fde22c5420e ("scsi: mpt3sas: Stop fw fault watchdog work item during system shutdown")
I still have questions about the other stuff that is skipped, or what
problem was actually being solved by skipping the dma pool
cleanup. Doesn't skipping that leave mappings behind in the iommu?
Is any of this memory a location that the firmware was trying to write
to before that is mentioned in the commit message?
Regards,
Jerry
> >
> > Regards,
> > Jerry
> >
>
