Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Salvatore Bonaccorso]

Dear stable maintainers,

Please backport the aboove commit to the stable series. Note, though
as a first step it just applies cleanly to 6.1.y. Due to 9df650963bf6
("scsi: mpt3sas: Don't change DMA mask while reallocating pools") it
does not apply cleanly to earlier series.

For context: There were several reports in Debian about regression in
5.10.y already:

https://bugs.debian.org/1022268
https://bugs.debian.org/1023183
https://bugs.debian.org/1025747
https://bugs.debian.org/1022126

https://lore.kernel.org/linux-scsi/Y1JkuKTjVYrOWbvm@eldamar.lan/ is
the initial reporting to upstream and later on brought as well to the
regression list:

https://lore.kernel.org/regressions/754b030c-ba14-167c-e2d0-2f4f5bf55e99@leemhuis.info/

Thorsten suggested to first get the patch applied at least in 6.1.y
but for further steps down we need help. Sreekanth and Martin is this
still on your radar? Help with getting this back to 5.10.y would be
welcome, and I'm sure with a tentative patch I can get some of the
reporting users to report a Tested-by.

While people are "unhappy" why it is not fixed yet in Debian, I'm not
willing to diverge and appy a patch which is not approved and aimed to
go upstream into the respective stable series. But I have too less
knowledge here to see which is the correct change to apply to back to
5.10.y alone in case we need to diverge, as 9df650963bf6 picking as
well is not an option. Input from you experts on that would be more
the appreciated.

Regards,
Salvatore

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Greg Kroah-Hartman]

On Sat, Mar 04, 2023 at 10:52:07AM +0100, Salvatore Bonaccorso wrote:
> Dear stable maintainers,
> 
> Please backport the aboove commit to the stable series. Note, though
> as a first step it just applies cleanly to 6.1.y. Due to 9df650963bf6
> ("scsi: mpt3sas: Don't change DMA mask while reallocating pools") it
> does not apply cleanly to earlier series.
> 
> For context: There were several reports in Debian about regression in
> 5.10.y already:
> 
> https://bugs.debian.org/1022268
> https://bugs.debian.org/1023183
> https://bugs.debian.org/1025747
> https://bugs.debian.org/1022126
> 
> https://lore.kernel.org/linux-scsi/Y1JkuKTjVYrOWbvm@eldamar.lan/ is
> the initial reporting to upstream and later on brought as well to the
> regression list:
> 
> https://lore.kernel.org/regressions/754b030c-ba14-167c-e2d0-2f4f5bf55e99@leemhuis.info/
> 
> Thorsten suggested to first get the patch applied at least in 6.1.y
> but for further steps down we need help. Sreekanth and Martin is this
> still on your radar? Help with getting this back to 5.10.y would be
> welcome, and I'm sure with a tentative patch I can get some of the
> reporting users to report a Tested-by.

It only applies to 6.1.y, I would need a working backport to apply it to
any older kernels.

thanks,

greg k-h

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Salvatore Bonaccorso]

Hi Greg,

On Mon, Mar 06, 2023 at 08:21:18AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Mar 04, 2023 at 10:52:07AM +0100, Salvatore Bonaccorso wrote:
> > Dear stable maintainers,
> > 
> > Please backport the aboove commit to the stable series. Note, though
> > as a first step it just applies cleanly to 6.1.y. Due to 9df650963bf6
> > ("scsi: mpt3sas: Don't change DMA mask while reallocating pools") it
> > does not apply cleanly to earlier series.
> > 
> > For context: There were several reports in Debian about regression in
> > 5.10.y already:
> > 
> > https://bugs.debian.org/1022268
> > https://bugs.debian.org/1023183
> > https://bugs.debian.org/1025747
> > https://bugs.debian.org/1022126
> > 
> > https://lore.kernel.org/linux-scsi/Y1JkuKTjVYrOWbvm@eldamar.lan/ is
> > the initial reporting to upstream and later on brought as well to the
> > regression list:
> > 
> > https://lore.kernel.org/regressions/754b030c-ba14-167c-e2d0-2f4f5bf55e99@leemhuis.info/
> > 
> > Thorsten suggested to first get the patch applied at least in 6.1.y
> > but for further steps down we need help. Sreekanth and Martin is this
> > still on your radar? Help with getting this back to 5.10.y would be
> > welcome, and I'm sure with a tentative patch I can get some of the
> > reporting users to report a Tested-by.
> 
> It only applies to 6.1.y, I would need a working backport to apply it to
> any older kernels.

Yes. Sorry if that was not clear from the above. Waiting fror input
from Sreekanth and Martin on it.

Regards,
Salvatore

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Martin K. Petersen]

Hi Salvatore!

> Sreekanth and Martin is this still on your radar?

Broadcom will have to provide a suitable fix for the relevant older
stable releases. It is the patch author's responsibility to provide
backports.

> as 9df650963bf6 picking as well is not an option.

Why not?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Salvatore Bonaccorso]

[-- Attachment #1: Type: text/plain, Size: 1242 bytes --]

hi Martin, hi Sreekanth,

On Mon, Mar 06, 2023 at 08:16:35PM -0500, Martin K. Petersen wrote:
> 
> Hi Salvatore!
> 
> > Sreekanth and Martin is this still on your radar?
> 
> Broadcom will have to provide a suitable fix for the relevant older
> stable releases. It is the patch author's responsibility to provide
> backports.
> 
> > as 9df650963bf6 picking as well is not an option.
> 
> Why not?

Thanks to Martin Wilck from SUSE to get me understanding the picture.
The problem is that e0e0747de0ea ("scsi: mpt3sas:
Fix return value check of dma_get_required_mask()") was backported to
several series. In mainline though the mis-merge did undo that. So I
believe the right thing would be to revert first in the stable series
where it was applied (5.10.y, 5.15.y) the commit e0e0747de0ea ("scsi:
mpt3sas: Fix return value check of dma_get_required_mask()")  and then
on top of this revert apply the patches:

9df650963bf6 ("scsi: mpt3sas: Don't change DMA mask while reallocating pools")
1a2dcbdde82e ("scsi: mpt3sas: re-do lost mpt3sas DMA mask fix")
06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API")

Attached mbox file implements this.

Does that looks now good for resolving the regression?

Regards,
Salvatore

[-- Attachment #2: mpt3sas-xen-regression.mbox --]
[-- Type: application/mbox, Size: 8128 bytes --]

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Martin Wilck]

On Wed, 2023-03-08 at 17:32 +0100, Salvatore Bonaccorso wrote:
> hi Martin, hi Sreekanth,
> 
> On Mon, Mar 06, 2023 at 08:16:35PM -0500, Martin K. Petersen wrote:
> > 
> > Hi Salvatore!
> > 
> > > Sreekanth and Martin is this still on your radar?
> > 
> > Broadcom will have to provide a suitable fix for the relevant older
> > stable releases. It is the patch author's responsibility to provide
> > backports.
> > 
> > > as 9df650963bf6 picking as well is not an option.
> > 
> > Why not?
> 
> Thanks to Martin Wilck from SUSE to get me understanding the picture.
> The problem is that e0e0747de0ea ("scsi: mpt3sas:
> Fix return value check of dma_get_required_mask()") was backported to
> several series. In mainline though the mis-merge did undo that. So I
> believe the right thing would be to revert first in the stable series
> where it was applied (5.10.y, 5.15.y) the commit e0e0747de0ea ("scsi:
> mpt3sas: Fix return value check of dma_get_required_mask()")  and
> then
> on top of this revert apply the patches:
> 
> 9df650963bf6 ("scsi: mpt3sas: Don't change DMA mask while
> reallocating pools")
> 1a2dcbdde82e ("scsi: mpt3sas: re-do lost mpt3sas DMA mask fix")
> 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask()
> API")
> 
> Attached mbox file implements this.
> 
> Does that looks now good for resolving the regression?
> 

Yes, this makes sense and it's actually the same thing I did for our
5.14 series. Thanks for re-figuring it out, the matter is really
confusing.

Regards,
Martin

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Salvatore Bonaccorso]

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

Hi Martin,

On Wed, Mar 08, 2023 at 07:06:10PM +0100, Martin Wilck wrote:
> On Wed, 2023-03-08 at 17:32 +0100, Salvatore Bonaccorso wrote:
> > hi Martin, hi Sreekanth,
> > 
> > On Mon, Mar 06, 2023 at 08:16:35PM -0500, Martin K. Petersen wrote:
> > > 
> > > Hi Salvatore!
> > > 
> > > > Sreekanth and Martin is this still on your radar?
> > > 
> > > Broadcom will have to provide a suitable fix for the relevant older
> > > stable releases. It is the patch author's responsibility to provide
> > > backports.
> > > 
> > > > as 9df650963bf6 picking as well is not an option.
> > > 
> > > Why not?
> > 
> > Thanks to Martin Wilck from SUSE to get me understanding the picture.
> > The problem is that e0e0747de0ea ("scsi: mpt3sas:
> > Fix return value check of dma_get_required_mask()") was backported to
> > several series. In mainline though the mis-merge did undo that. So I
> > believe the right thing would be to revert first in the stable series
> > where it was applied (5.10.y, 5.15.y) the commit e0e0747de0ea ("scsi:
> > mpt3sas: Fix return value check of dma_get_required_mask()")  and
> > then
> > on top of this revert apply the patches:
> > 
> > 9df650963bf6 ("scsi: mpt3sas: Don't change DMA mask while
> > reallocating pools")
> > 1a2dcbdde82e ("scsi: mpt3sas: re-do lost mpt3sas DMA mask fix")
> > 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask()
> > API")
> > 
> > Attached mbox file implements this.
> > 
> > Does that looks now good for resolving the regression?
> > 
> 
> Yes, this makes sense and it's actually the same thing I did for our
> 5.14 series. Thanks for re-figuring it out, the matter is really
> confusing.

Thanks for confirming. 

I had a small typo in the commit message of the revert commit,
attached is an updated mbox with that fixed (afferomentioned ->
aforementioned).

Regards,
Salvatore

[-- Attachment #2: mpt3sas-xen-regression.mbox --]
[-- Type: application/mbox, Size: 8127 bytes --]

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Greg Kroah-Hartman]

On Wed, Mar 08, 2023 at 08:33:21PM +0100, Salvatore Bonaccorso wrote:
> Hi Martin,
> 
> On Wed, Mar 08, 2023 at 07:06:10PM +0100, Martin Wilck wrote:
> > On Wed, 2023-03-08 at 17:32 +0100, Salvatore Bonaccorso wrote:
> > > hi Martin, hi Sreekanth,
> > > 
> > > On Mon, Mar 06, 2023 at 08:16:35PM -0500, Martin K. Petersen wrote:
> > > > 
> > > > Hi Salvatore!
> > > > 
> > > > > Sreekanth and Martin is this still on your radar?
> > > > 
> > > > Broadcom will have to provide a suitable fix for the relevant older
> > > > stable releases. It is the patch author's responsibility to provide
> > > > backports.
> > > > 
> > > > > as 9df650963bf6 picking as well is not an option.
> > > > 
> > > > Why not?
> > > 
> > > Thanks to Martin Wilck from SUSE to get me understanding the picture.
> > > The problem is that e0e0747de0ea ("scsi: mpt3sas:
> > > Fix return value check of dma_get_required_mask()") was backported to
> > > several series. In mainline though the mis-merge did undo that. So I
> > > believe the right thing would be to revert first in the stable series
> > > where it was applied (5.10.y, 5.15.y) the commit e0e0747de0ea ("scsi:
> > > mpt3sas: Fix return value check of dma_get_required_mask()")  and
> > > then
> > > on top of this revert apply the patches:
> > > 
> > > 9df650963bf6 ("scsi: mpt3sas: Don't change DMA mask while
> > > reallocating pools")
> > > 1a2dcbdde82e ("scsi: mpt3sas: re-do lost mpt3sas DMA mask fix")
> > > 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask()
> > > API")
> > > 
> > > Attached mbox file implements this.
> > > 
> > > Does that looks now good for resolving the regression?
> > > 
> > 
> > Yes, this makes sense and it's actually the same thing I did for our
> > 5.14 series. Thanks for re-figuring it out, the matter is really
> > confusing.
> 
> Thanks for confirming. 
> 
> I had a small typo in the commit message of the revert commit,
> attached is an updated mbox with that fixed (afferomentioned ->
> aforementioned).

Now queued up for 5.10.y and 5.15.y, thanks!

greg k-h

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Martin K. Petersen]

Salvatore,

> So I believe the right thing would be to revert first in the stable
> series where it was applied (5.10.y, 5.15.y) the commit e0e0747de0ea
> ("scsi: mpt3sas: Fix return value check of dma_get_required_mask()")
> and then on top of this revert apply the patches:
>
> 9df650963bf6 ("scsi: mpt3sas: Don't change DMA mask while reallocating pools")
> 1a2dcbdde82e ("scsi: mpt3sas: re-do lost mpt3sas DMA mask fix")
> 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API")
>
> Attached mbox file implements this.
>
> Does that looks now good for resolving the regression?

Yes, that's one way to resolve it.

At a quick glance your mbox looks fine. Best way to validate would be to
compare the resulting _base_config_dma_addressing() function between
your tree and upstream. I don't believe we have had additional changes
here so there should be no delta.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Salvatore Bonaccorso]

Hi Martin,

On Wed, Mar 08, 2023 at 02:05:01PM -0500, Martin K. Petersen wrote:
> 
> Salvatore,
> 
> > So I believe the right thing would be to revert first in the stable
> > series where it was applied (5.10.y, 5.15.y) the commit e0e0747de0ea
> > ("scsi: mpt3sas: Fix return value check of dma_get_required_mask()")
> > and then on top of this revert apply the patches:
> >
> > 9df650963bf6 ("scsi: mpt3sas: Don't change DMA mask while reallocating pools")
> > 1a2dcbdde82e ("scsi: mpt3sas: re-do lost mpt3sas DMA mask fix")
> > 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API")
> >
> > Attached mbox file implements this.
> >
> > Does that looks now good for resolving the regression?
> 
> Yes, that's one way to resolve it.
> 
> At a quick glance your mbox looks fine. Best way to validate would be to
> compare the resulting _base_config_dma_addressing() function between
> your tree and upstream. I don't believe we have had additional changes
> here so there should be no delta.

Yes, the resulting _base_config_dma_addressing() function is the same
after applying the series of commits.

In both cases it will be:

/**
 * _base_config_dma_addressing - set dma addressing
 * @ioc: per adapter object
 * @pdev: PCI device struct
 *
 * Return: 0 for success, non-zero for failure.
 */
static int
_base_config_dma_addressing(struct MPT3SAS_ADAPTER *ioc, struct pci_dev *pdev)
{
        struct sysinfo s;
        u64 coherent_dma_mask, dma_mask;

        if (ioc->is_mcpu_endpoint || sizeof(dma_addr_t) == 4) {
                ioc->dma_mask = 32;
                coherent_dma_mask = dma_mask = DMA_BIT_MASK(32);
        /* Set 63 bit DMA mask for all SAS3 and SAS35 controllers */
        } else if (ioc->hba_mpi_version_belonged > MPI2_VERSION) {
                ioc->dma_mask = 63;
                coherent_dma_mask = dma_mask = DMA_BIT_MASK(63);
        } else {
                ioc->dma_mask = 64;
                coherent_dma_mask = dma_mask = DMA_BIT_MASK(64);
        }

        if (ioc->use_32bit_dma)
                coherent_dma_mask = DMA_BIT_MASK(32);

        if (dma_set_mask(&pdev->dev, dma_mask) ||
            dma_set_coherent_mask(&pdev->dev, coherent_dma_mask))
                return -ENODEV;

        if (ioc->dma_mask > 32) {
                ioc->base_add_sg_single = &_base_add_sg_single_64;
                ioc->sge_size = sizeof(Mpi2SGESimple64_t);
        } else {
                ioc->base_add_sg_single = &_base_add_sg_single_32;
                ioc->sge_size = sizeof(Mpi2SGESimple32_t);
        }

        si_meminfo(&s);
        ioc_info(ioc, "%d BIT PCI BUS DMA ADDRESSING SUPPORTED, total mem (%ld kB)\n",
                ioc->dma_mask, convert_to_kb(s.totalram));

        return 0;
}

Regards,
Salvatore

Re: Please apply commit 06e472acf964 ("scsi: mpt3sas: Remove usage of dma_get_required_mask() API") to stable series

[Martin K. Petersen]

Salvatore,

>> At a quick glance your mbox looks fine. Best way to validate would be
>> to compare the resulting _base_config_dma_addressing() function
>> between your tree and upstream. I don't believe we have had
>> additional changes here so there should be no delta.

> Yes, the resulting _base_config_dma_addressing() function is the same
> after applying the series of commits.

That looks good to me.

-- 
Martin K. Petersen	Oracle Linux Engineering