On 2020-06-21 21:20, David Rientjes wrote:
On Sun, 21 Jun 2020, Guenter Roeck wrote:
This patch results in a boot failure in some of my powerpc boot tests,
specifically those testing boots from mptsas1068 devices. Error message:
mptsas 0000:00:02.0: enabling device (0000 -> 0002)
mptbase: ioc0: Initiating bringup
ioc0: LSISAS1068 A0: Capabilities={Initiator}
mptbase: ioc0: ERROR - Unable to allocate Reply, Request, Chain Buffers!
mptbase: ioc0: ERROR - didn't initialize properly! (-3)
mptsas: probe of 0000:00:02.0 failed with error -3
Configuration is bamboo:44x/bamboo_defconfig plus various added drivers.
Qemu command line is
qemu-system-ppc -kernel vmlinux -M bamboo \
-m 256 -no-reboot -snapshot -device mptsas1068,id=scsi \
-device scsi-hd,bus=scsi.0,drive=d0,wwn=0x5000c50015ea71ac -drive \
file=rootfs.ext2,format=raw,if=none,id=d0 \
--append "panic=-1 slub_debug=FZPUA root=/dev/sda mem=256M
console=ttyS0" \
-monitor none -nographic
canyonlands_defconfig with sam460ex machine and otherwise similar command line
fails as well.
Reverting this patch fixes the problem.
This looks like the minimum value of 128 KiB is not sufficient, and the
bug is in the intention of 1d659236fb43c4d2 ("dma-pool: scale the
default DMA coherent pool size with memory capacity")?
Before, there was a single pool of (fixed) 256 KiB size, now there are
up to three coherent pools (DMA, DMA32, and kernel), albeit of smaller
size (128 KiB each).
Can you print the requested size in drivers/message/fusion/mptbase.c:
PrimeIocFifos()?
172928 bytes
Does replacing all SZ_128K by SZ_256K in my patch help?
Yes, it does.
The new coherent pools should auto expand when they are close to being
depleted but there's no guarantee that it can be done fast enough.
More to the point, it's never going to help if the pool is empty and one
allocation is simply larger than the entire thing ;)
Another angle, though, is to question why this driver is making such a
large allocation with GFP_ATOMIC in the first place. At a glance it
looks like there's no reason at all other than that it's still using the
legacy pci_alloc_consistent() API, since every path to that appears to
have CAN_SLEEP passed as its flag - modernising that would arguably be
an even better long-term win.
Robin.
Switching the min size to be the previous min size (256KB) seems like the
best option and it matches what
Documentation/admin-guide/kernel-parameters.txt still stays.
I'll also send a patch to point in the right direction when this happens.
