On 4/11/23 17:43, Jerry Snitselaar wrote:
> 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.
Can this do any harm? If no then it is pointless.
>
> 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
>>>
>>
>
