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

DISCUSSION Salvatore Bonaccorso 4 participants 11 messages Mar 4, 2023 – Mar 10, 2023 Stable branch

Share & tools

Salvatore Bonaccorso

author

2023-03-04 09:52 UTC #0 lore raw

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/[email protected]/ is the initial reporting to upstream and later on brought as well to the regression list:

https://lore.kernel.org/regressions/[email protected]/

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

Greg Kroah-Hartman

maintainer

2023-03-06 07:21 UTC #1 lore raw

Quoted 19 lines · 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/[email protected]/ is the initial reporting to upstream and later on brought as well to the regression list:

Quoted 7 lines

https://lore.kernel.org/regressions/[email protected]/
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

Salvatore Bonaccorso

author

2023-03-06 20:53 UTC #2 lore raw

Hi Greg,

Quoted 22 lines · Greg Kroah-Hartman

Quoted 21 lines · 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/[email protected]/ is the initial reporting to upstream and later on brought as well to the regression list:
https://lore.kernel.org/regressions/[email protected]/

Quoted 8 lines

Quoted 5 lines
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

Martin K. Petersen

2023-03-07 01:17 UTC #3 lore raw

In reply to Salvatore Bonaccorso

Hi Salvatore!

Quoted 1 line

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.

Quoted 1 line

as 9df650963bf6 picking as well is not an option.

Why not?

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Salvatore Bonaccorso

author

2023-03-08 16:32 UTC #4 lore raw

hi Martin, hi Sreekanth,

Quoted 5 lines · Martin K. Petersen

Hi Salvatore!
Quoted 1 line
Sreekanth and Martin is this still on your radar?

Quoted 7 lines

Broadcom will have to provide a suitable fix for the relevant older stable releases. It is the patch author's responsibility to provide backports.
Quoted 1 line
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

Martin Wilck

2023-03-08 18:06 UTC #5 lore raw

Quoted 32 lines · Salvatore Bonaccorso

hi Martin, hi Sreekanth,
Quoted 5 lines · Martin K. Petersen
Hi Salvatore!
Quoted 1 line
Sreekanth and Martin is this still on your radar?
Quoted 7 lines
Broadcom will have to provide a suitable fix for the relevant older stable releases. It is the patch author's responsibility to provide backports.
Quoted 1 line
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")

Quoted 4 lines

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

Salvatore Bonaccorso

author

2023-03-08 19:33 UTC #6 lore raw

Hi Martin,

Quoted 33 lines · Martin Wilck

Quoted 32 lines · Salvatore Bonaccorso
hi Martin, hi Sreekanth,
Quoted 5 lines · Martin K. Petersen
Hi Salvatore!
Quoted 1 line
Sreekanth and Martin is this still on your radar?
Quoted 7 lines
Broadcom will have to provide a suitable fix for the relevant older stable releases. It is the patch author's responsibility to provide backports.
Quoted 1 line
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")

Quoted 8 lines

Quoted 4 lines
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

Greg Kroah-Hartman

maintainer

2023-03-10 12:54 UTC #7 lore raw

Quoted 45 lines · Salvatore Bonaccorso

Hi Martin,
Quoted 33 lines · Martin Wilck
Quoted 32 lines · Salvatore Bonaccorso
hi Martin, hi Sreekanth,
Quoted 5 lines · Martin K. Petersen
Hi Salvatore!
Quoted 1 line
Sreekanth and Martin is this still on your radar?
Quoted 7 lines
Broadcom will have to provide a suitable fix for the relevant older stable releases. It is the patch author's responsibility to provide backports.
Quoted 1 line
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")
Quoted 8 lines
Quoted 4 lines
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.

Quoted 5 lines

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

Martin K. Petersen

2023-03-08 19:07 UTC #8 lore raw

In reply to Salvatore Bonaccorso

Salvatore,

Quoted 5 lines

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:

Quoted 7 lines

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.

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Salvatore Bonaccorso

author

2023-03-09 09:47 UTC #9 lore raw

Hi Martin,

Quoted 14 lines · Martin K. Petersen

Salvatore,
Quoted 11 lines
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.

Quoted 8 lines

Quoted 1 line
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

Martin K. Petersen

2023-03-09 15:52 UTC #10 lore raw

Salvatore,

Quoted 7 lines

Quoted 4 lines
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.

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering