dcbz instruction shouldn't be used on non-cached memory. Using
it on non-cached memory can result in alignment exception and
implies a heavy handling.
Instead of silentely emulating the instruction and resulting in high
performance degradation, warn whenever an alignment exception is
taken in kernel mode due to dcbz, so that the user is made aware that
dcbz instruction has been used unexpectedly by the kernel.
Reported-by: Stan Johnson <redacted>
Cc: Finn Thain <fthain@linux-m68k.org>
Signed-off-by: Christophe Leroy <redacted>
---
v2: Warn only when emulating kernel mode
---
arch/powerpc/kernel/align.c | 1 +
1 file changed, 1 insertion(+)
From: Michael Ellerman <hidden> Date: 2021-11-02 11:39:02
On Thu, 16 Sep 2021 16:52:09 +0200, Christophe Leroy wrote:
dcbz instruction shouldn't be used on non-cached memory. Using
it on non-cached memory can result in alignment exception and
implies a heavy handling.
Instead of silentely emulating the instruction and resulting in high
performance degradation, warn whenever an alignment exception is
taken in kernel mode due to dcbz, so that the user is made aware that
dcbz instruction has been used unexpectedly by the kernel.
[...]
From: Christian Lamparter <hidden> Date: 2024-08-21 22:48:52
Sorry to write a reply to this old mail. But after years, I finally
decided to tackle an "old" problem that has come up...
And unfortunately it is related to this patch. But let me explain.
On 9/16/21 4:52 PM, Christophe Leroy wrote:
dcbz instruction shouldn't be used on non-cached memory. Using
it on non-cached memory can result in alignment exception and
implies a heavy handling.
Instead of silentely emulating the instruction and resulting in high
performance degradation, warn whenever an alignment exception is
taken in kernel mode due to dcbz, so that the user is made aware that
dcbz instruction has been used unexpectedly by the kernel.
The devices that are affected are APM821xx: MyBook Live, Meraki MX60, Netgear WNDR4700, ...
All these have this "splat" during boot:
|[ 8.777686] PPC 4xx OCP EMAC driver, version 3.54 [ 8.782961] ------------[ cut here ]------------ [ 8.787565] WARNING: CPU: 0 PID: 1 at fix_alignment+0x144/0x16c [ 8.793474] Modules linked in: [ 8.796527] CPU: 0 PID: 1 Comm: swapper Not tainted 6.6.47 #0 [ 8.802253] Hardware name: Meraki MX60/MX60W Security Appliance APM821XX 0x12c41c83 PowerPC 44x Platform [ 8.811688] NIP: c0003158 LR: c0003070 CTR: c001471c [ 8.816725] REGS: c10319d0 TRAP: 0700 Not tainted (6.6.47) [ 8.822453] MSR: 00029000 <CE,EE,ME> CR: 24000442 XER: 20000000 [ 8.828630] [ 8.828630] GPR00: c000855c c1031ac0 c1051c00 00000000 3e039bf6 00000018 01f01cdf 00000004 [ 8.828630] GPR08: 0000000f 00000000 00000510 c1031b30 c0013a94 00000000 c0002518 00000000 [ 8.828630] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 8.828630] GPR24: 00000000 dfb953ac c00790c8 c104e120 c10f5610 00001000
c0d30000 c1031b40 [ 8.863432] NIP [c0003158] fix_alignment+0x144/0x16c [ 8.868384] LR [c0003070] fix_alignment+0x5c/0x16c [ 8.873162] Call Trace: [ 8.875598] [c1031ac0] [c0186efc] get_page_from_freelist+0x208/0xdac (unreliable) [ 8.883063] [c1031b10] [c000855c] alignment_exception+0xf0/0x164 [ 8.889058] [c1031b30] [c0000a30] Alignment+0x130/0x180 [ 8.894269] --- interrupt: 600 at memset+0x60/0xc0 [ 8.899063] NIP: c0013a94 LR: c007a1ac CTR: 0000007f [ 8.904092] REGS: c1031b40 TRAP: 0600 Not tainted (6.6.47) [ 8.909812] MSR: 00029000 <CE,EE,ME> CR: 44000442 XER: 20000000 [ 8.915989] DEAR: e1168020 ESR: 00000000 [ 8.915989] GPR00: 00000007 c1031c30 c1051c00 e1168000 00000000 00001000 e116801c 00000004 [ 8.915989] GPR08: 00001000 0000007f 00000a3a 00000000 24000848 00000000 c0002518 00000000 [ 8.915989] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [
8.915989] GPR24: 00000000 dfb953ac c00790c8 c104e120 c10f5610 00001000 c0d30000 e1168000 [ 8.953306] NIP [c0013a94] memset+0x60/0xc0 [ 8.957478] LR [c007a1ac] dma_direct_alloc+0x1e4/0x290 [ 8.962620] --- interrupt: 600 [ 8.965661] [c1031c30] [c007a130] dma_direct_alloc+0x168/0x290 (unreliable) [ 8.972608] [c1031c70] [c00790c8] dma_alloc_attrs+0xa8/0xf8 [ 8.978163] [c1031cb0] [c04d419c] mal_probe+0x26c/0x664 [ 8.983390] [c1031ce0] [c04326d8] platform_probe+0x60/0xb0 [ 8.988867] [c1031d00] [c042f6f0] really_probe+0xd8/0x3b4 [ 8.994251] [c1031d30] [c042fbdc] driver_probe_device+0x48/0x104 [ 9.000238] [c1031d60] [c042fe7c] __driver_attach+0xb4/0x1a4 [ 9.005880] [c1031d80] [c042cdc8] bus_for_each_dev+0x7c/0xc8 [ 9.011548] [c1031db0] [c042e708] bus_add_driver+0x170/0x25c [ 9.017199] [c1031de0] [c0430cec] driver_register+0x8c/0x164 [ 9.022840] [c1031e00] [c0902de0] emac_init+0x17c/0x1ec [
9.028067] [c1031e40] [c0002680] do_one_initcall+0x50/0x2ac [ 9.033709] [c1031eb0] [c08e60f8] kernel_init_freeable+0x1b0/0x230 [ 9.039870] [c1031ee0] [c0002538] kernel_init+0x20/0x118 [ 9.045166] [c1031f00] [c000d1f8] ret_from_kernel_user_thread+0x10/0x18 [ 9.051758] --- interrupt: 0 at 0x0 [ 9.055241] Code: 39400000 7d500ba6 4c00012c 2c090000 41a2fefc 83e1004c 3860fff2 38210050 4e800020 38a00000 3920fff2 4bffffd4 <0fe00000> 4bffff8c 80010054 7c0803a6 [ 9.069929] ---[ end trace 0000000000000000 ]---|
what happens is that the EMAC (ethernet driver) uses a dma_alloc_coherent here:
<https://elixir.bootlin.com/linux/v6.6.47/source/drivers/net/ethernet/ibm/emac/mal.c#L636>
| bd_size <https://elixir.bootlin.com/linux/v6.6.47/C/ident/bd_size>=sizeof(structmal_descriptor <https://elixir.bootlin.com/linux/v6.6.47/C/ident/mal_descriptor>)*
| (NUM_TX_BUFF <https://elixir.bootlin.com/linux/v6.6.47/C/ident/NUM_TX_BUFF>*mal <https://elixir.bootlin.com/linux/v6.6.47/C/ident/mal>->num_tx_chans <https://elixir.bootlin.com/linux/v6.6.47/C/ident/num_tx_chans>+
| NUM_RX_BUFF <https://elixir.bootlin.com/linux/v6.6.47/C/ident/NUM_RX_BUFF>*mal <https://elixir.bootlin.com/linux/v6.6.47/C/ident/mal>->num_rx_chans <https://elixir.bootlin.com/linux/v6.6.47/C/ident/num_rx_chans>);
| mal <https://elixir.bootlin.com/linux/v6.6.47/C/ident/mal>->bd_virt <https://elixir.bootlin.com/linux/v6.6.47/C/ident/bd_virt>=dma_alloc_coherent <https://elixir.bootlin.com/linux/v6.6.47/C/ident/dma_alloc_coherent>(&ofdev <https://elixir.bootlin.com/linux/v6.6.47/C/ident/ofdev>->dev,bd_size <https://elixir.bootlin.com/linux/v6.6.47/C/ident/bd_size>,&mal <https://elixir.bootlin.com/linux/v6.6.47/C/ident/mal>->bd_dma <https://elixir.bootlin.com/linux/v6.6.47/C/ident/bd_dma>,
| GFP_KERNEL <https://elixir.bootlin.com/linux/v6.6.47/C/ident/GFP_KERNEL>);
|
(bd_size is 2x1024 = 2048)
and this results in a call to dma_direct_allocation(), which has one
innocent looking memset():
<https://elixir.bootlin.com/linux/v6.6.47/source/kernel/dma/direct.c#L301>
which zeros out the area before returning it back...
And of course, this triggers the WARNING above. As for why memset uses
that dcbz... this is explained in:
<https://elixir.bootlin.com/linux/v6.6.47/source/arch/powerpc/lib/copy_32.S#L86>
| * Use dcbz on the complete cache lines in the destination
| * to set them to zero. This requires that the destination
| * area is cacheable. -- paulus
| *
| * During early init, cache might not be active yet, so dcbz cannot be used.
| * We therefore skip the optimised bloc that uses dcbz. This jump is
| * replaced by a nop once cache is active. This is done in machine_init()
could this WARN_ON_ONCE maybe be downgraded to a single pr_info_once?
I don't see how to tackle this "neatly" in a better way.
BugLink: <https://github.com/openwrt/openwrt/pull/14037#issuecomment-2302485414>
Best Regards,
Christian
quoted hunk
Reported-by: Stan Johnson <redacted>
Cc: Finn Thain <fthain@linux-m68k.org>
Signed-off-by: Christophe Leroy <redacted>
---
v2: Warn only when emulating kernel mode
---
arch/powerpc/kernel/align.c | 1 +
1 file changed, 1 insertion(+)
+ CC Christoph Hellwig
Hi,
Le 22/08/2024 à 00:39, Christian Lamparter a écrit :
[Vous ne recevez pas souvent de courriers de
christian.lamparter@isd.uni-stuttgart.de. Découvrez pourquoi ceci est
important à https://aka.ms/LearnAboutSenderIdentification ]
Sorry to write a reply to this old mail. But after years, I finally
decided to tackle an "old" problem that has come up...
And unfortunately it is related to this patch. But let me explain.
On 9/16/21 4:52 PM, Christophe Leroy wrote:
quoted
dcbz instruction shouldn't be used on non-cached memory. Using
it on non-cached memory can result in alignment exception and
implies a heavy handling.
Instead of silentely emulating the instruction and resulting in high
performance degradation, warn whenever an alignment exception is
taken in kernel mode due to dcbz, so that the user is made aware that
dcbz instruction has been used unexpectedly by the kernel.
The devices that are affected are APM821xx: MyBook Live, Meraki MX60,
Netgear WNDR4700, ...
All these have this "splat" during boot:
...
what happens is that the EMAC (ethernet driver) uses a
dma_alloc_coherent here:
...
and this results in a call to dma_direct_allocation(), which has one
innocent looking memset():
memset() can't be used on non-cached memory, memset_io() has to be used
instead.
...
And of course, this triggers the WARNING above. As for why memset uses
that dcbz... this is explained in:
...
could this WARN_ON_ONCE maybe be downgraded to a single pr_info_once?
I don't see how to tackle this "neatly" in a better way.
Le 22/08/2024 à 07:32, Christoph Hellwig a écrit :
On Thu, Aug 22, 2024 at 05:25:10AM +0000, LEROY Christophe wrote:
quoted
quoted
and this results in a call to dma_direct_allocation(), which has one
innocent looking memset():
memset() can't be used on non-cached memory, memset_io() has to be used
instead.
No, we use memset on uncached memory all the time. Note that uncached
memory != __iomem memory, for which you DO have to use memset_io.
Then we have a subject here.
powerpc has a magic instruction 'dcbz' which clears a full cacheline in
one go. It is far more efficient than a loop to store zeros, and since
2015 memset(0) has been implemented with that instruction (commit
5b2a32e80634 ("powerpc/32: memset(0): use cacheable_memzero"))
But that instruction generates an alignment exception when used on
non-cached memory (whether it is RAM or not doesn't matter). It is then
emulated by the kernel but it of course leads to a serious performance
degradation, hence the warning added by commit cbe654c77961 ("powerpc:
warn on emulation of dcbz instruction in kernel mode"). Until now it
helped identify and fix use of memset() on IO memory.
But if memset() is expected to be used with non-cached RAM, then I don't
know what to do. Any suggestion ?
Christophe
From: Christoph Hellwig <hch@lst.de> Date: 2024-08-22 07:14:48
On Thu, Aug 22, 2024 at 06:39:33AM +0000, LEROY Christophe wrote:
powerpc has a magic instruction 'dcbz' which clears a full cacheline in
one go. It is far more efficient than a loop to store zeros, and since
2015 memset(0) has been implemented with that instruction (commit
5b2a32e80634 ("powerpc/32: memset(0): use cacheable_memzero"))
But that instruction generates an alignment exception when used on
non-cached memory (whether it is RAM or not doesn't matter). It is then
emulated by the kernel but it of course leads to a serious performance
degradation, hence the warning added by commit cbe654c77961 ("powerpc:
warn on emulation of dcbz instruction in kernel mode"). Until now it
helped identify and fix use of memset() on IO memory.
But if memset() is expected to be used with non-cached RAM, then I don't
know what to do. Any suggestion ?
I'd suggest two things:
1) remove the warning. The use case is perfectly valid and everything
using uncached memory is already slow, so people will just have to
deal with it. Maybe offer a trace point instead if people care about
it.
2) figure out a way to avoid this case in the dma-coherent allocator,
which is probably the only case where it happens frequently
(a few drivers also zero or re-zero coherent memory, but most of the
time that is cargo cult programming and not actually needed)
For 2 I can think of two options:
a) provide a arch hook for zeroing the dma memory that defaults to
memset, but which powerpc can override
a) figure out a way to clear the memory before marking it uncached
if we can
a) it obviously easier to verify, but b) is probably going to give
way better performance.
Below is an untested implementation of b) for dma-direct, I just need to
find out if there is any architecture that requires the memory to be
zeroed after it іt has been remapped. The iommu drivers might also
need similar treatment.
@@ -275,6 +275,9 @@ void *dma_direct_alloc(struct device *dev, size_t size,if(force_dma_unencrypted(dev))prot=pgprot_decrypted(prot);+if(!PageHighMem(page))+memset(page_address(page),0,size);+/* remove any dirty cache lines on the kernel alias */arch_dma_prep_coherent(page,size);
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2024-08-22 10:24:09
On Thu, 2024-08-22 at 09:14 +0200, Christoph Hellwig wrote:
I'd suggest two things:
1) remove the warning. The use case is perfectly valid and everything
using uncached memory is already slow, so people will just have to
deal with it. Maybe offer a trace point instead if people care about
it.
Well... there's slow and extremely slow .. :-) dcbz() on uncached
memory will take an exception for every "cache line" of zeros which
will then need to be emulated. We are talking about hundreds to
thousands of cycle per cache line.
(Do we have an optimisation to detect memset in the emulation code and
force it to return to a non-dcbz'ing version ? If not, that's doable
and would at least limit it to one exception per memset() instead of
one per cache-line).
Cheers,
Ben.
From: Christian Lamparter <hidden> Date: 2024-08-22 18:19:40
Hi!
Thank you all for your insightful insights :-) .
... and derp, used the wrong mail.
On 8/22/24 9:14 AM, Christoph Hellwig wrote:
On Thu, Aug 22, 2024 at 06:39:33AM +0000, LEROY Christophe wrote:
quoted
powerpc has a magic instruction 'dcbz' which clears a full cacheline in
one go. It is far more efficient than a loop to store zeros, and since
2015 memset(0) has been implemented with that instruction (commit
5b2a32e80634 ("powerpc/32: memset(0): use cacheable_memzero"))
But that instruction generates an alignment exception when used on
non-cached memory (whether it is RAM or not doesn't matter). It is then
emulated by the kernel but it of course leads to a serious performance
degradation, hence the warning added by commit cbe654c77961 ("powerpc:
warn on emulation of dcbz instruction in kernel mode"). Until now it
helped identify and fix use of memset() on IO memory.
But if memset() is expected to be used with non-cached RAM, then I don't
know what to do. Any suggestion ?
I'd suggest two things:
1) remove the warning. The use case is perfectly valid and everything
using uncached memory is already slow, so people will just have to
deal with it. Maybe offer a trace point instead if people care about
it.
2) figure out a way to avoid this case in the dma-coherent allocator,
which is probably the only case where it happens frequently
(a few drivers also zero or re-zero coherent memory, but most of the
time that is cargo cult programming and not actually needed)
I tested your patch below and got the next warning. This time from
dma_alloc_from_pool about a similar memset in:
https://elixir.bootlin.com/linux/v6.6.47/source/kernel/dma/pool.c#L261
it triggers for the sata_dwc_ex460 driver when it access the harddrive.
For anyone interested, please look below for the full warning splat.
(I switched to the MyBook Live, it's easier to disassemble+reassemble
than the MX60... but with similar results).
quoted hunk
For 2 I can think of two options:
a) provide a arch hook for zeroing the dma memory that defaults to
memset, but which powerpc can override
a) figure out a way to clear the memory before marking it uncached
if we can
a) it obviously easier to verify, but b) is probably going to give
way better performance.
Below is an untested implementation of b) for dma-direct, I just need to
find out if there is any architecture that requires the memory to be
zeroed after it іt has been remapped. The iommu drivers might also
need similar treatment.
@@ -275,6 +275,9 @@ void *dma_direct_alloc(struct device *dev, size_t size,if(force_dma_unencrypted(dev))prot=pgprot_decrypted(prot);+if(!PageHighMem(page))+memset(page_address(page),0,size);+/* remove any dirty cache lines on the kernel alias */arch_dma_prep_coherent(page,size);
From: Christoph Hellwig <hch@lst.de> Date: 2024-08-23 08:07:42
dma_alloc_from_pool is the allocator used when the caller can't
sleep, and as that is reusing memory it really has to call memset
or a memset-like function on the already uncached memory
unfortunately. The dma engine operation that is doing this allocation
is documented as not being able to sleep, which is a bit unfortunate
as the storage driver above it could sleep just fine.
Adding the dmaengine maintainer and list if there is a way to pass a gfp
flag or some other indicator that the implementations can sleep,
which would avoid the need to use the not very scalable dma pool,
and thus also the need to memset on uncached memory.
Hi!
On Thu, Aug 22, 2024 at 06:39:33AM +0000, LEROY Christophe wrote:
Le 22/08/2024 à 07:32, Christoph Hellwig a écrit :
quoted
On Thu, Aug 22, 2024 at 05:25:10AM +0000, LEROY Christophe wrote:
quoted
quoted
and this results in a call to dma_direct_allocation(), which has one
innocent looking memset():
memset() can't be used on non-cached memory, memset_io() has to be used
instead.
No, we use memset on uncached memory all the time. Note that uncached
memory != __iomem memory, for which you DO have to use memset_io.
Then we have a subject here.
powerpc has a magic instruction 'dcbz' which clears a full cacheline in
one go. It is far more efficient than a loop to store zeros, and since
2015 memset(0) has been implemented with that instruction (commit
5b2a32e80634 ("powerpc/32: memset(0): use cacheable_memzero"))
But that instruction generates an alignment exception when used on
non-cached memory (whether it is RAM or not doesn't matter).
What does "uncached memory" even mean here? Literally it would be
I=1 memory (uncachEABLE memory), but more likely you want M=0 memory
here ("non-memory memory", "not well-behaved memory", MMIO often).
M=0 memory shouldn't ever have memset done on it, that is insane. And
I=1 memory should not have the same optimised routines used, since
those only make things slower still.
It is then
emulated by the kernel but it of course leads to a serious performance
degradation, hence the warning added by commit cbe654c77961 ("powerpc:
warn on emulation of dcbz instruction in kernel mode"). Until now it
helped identify and fix use of memset() on IO memory.
But if memset() is expected to be used with non-cached RAM, then I don't
know what to do. Any suggestion ?
If memset() is expected to be used with M=0, you cannot do any serious
optimisations to it at all. If memset() is expected to be used with I=1
it should use a separate code path for it, probably the caller should
make the distinction.
Segher
From: Christoph Hellwig <hch@lst.de> Date: 2024-08-23 13:55:03
On Fri, Aug 23, 2024 at 08:06:00AM -0500, Segher Boessenkool wrote:
What does "uncached memory" even mean here? Literally it would be
I=1 memory (uncachEABLE memory), but more likely you want M=0 memory
here ("non-memory memory", "not well-behaved memory", MMIO often).
Regular kernel memory vmapped with pgprot_noncached().
If memset() is expected to be used with M=0, you cannot do any serious
optimisations to it at all. If memset() is expected to be used with I=1
it should use a separate code path for it, probably the caller should
make the distinction.
DMA coherent memory which uses uncached memory for platforms that
do not provide hardware dma coherence can end up just about anywhere
in the kernel. We could use special routines for a few places in
the DMA subsystem, but there might be plenty of others.
Hi!
On Fri, Aug 23, 2024 at 03:54:59PM +0200, Christoph Hellwig wrote:
On Fri, Aug 23, 2024 at 08:06:00AM -0500, Segher Boessenkool wrote:
quoted
What does "uncached memory" even mean here? Literally it would be
I=1 memory (uncachEABLE memory), but more likely you want M=0 memory
here ("non-memory memory", "not well-behaved memory", MMIO often).
Regular kernel memory vmapped with pgprot_noncached().
So, I=1 (and G=1). Caching inhibited and guarded. But M=1 (memory
coherence required) as with any other real memory :-)
quoted
If memset() is expected to be used with M=0, you cannot do any serious
optimisations to it at all. If memset() is expected to be used with I=1
it should use a separate code path for it, probably the caller should
make the distinction.
DMA coherent memory which uses uncached memory for platforms that
do not provide hardware dma coherence can end up just about anywhere
in the kernel. We could use special routines for a few places in
the DMA subsystem, but there might be plenty of others.
Yeah. It will just be plenty slow, as we see here, that's what the
warning is for; but it works just fine :-)
The memset() code itself could chech for the storage attributes, but
that is probably more expensive than just assuming the happy case.
Maybe someone could try it out though!
Segher
From: Christian Lamparter <hidden> Date: 2024-08-23 23:43:24
On 8/23/24 9:19 PM, Segher Boessenkool wrote:
Hi!
On Fri, Aug 23, 2024 at 03:54:59PM +0200, Christoph Hellwig wrote:
quoted
On Fri, Aug 23, 2024 at 08:06:00AM -0500, Segher Boessenkool wrote:
quoted
What does "uncached memory" even mean here? Literally it would be
I=1 memory (uncachEABLE memory), but more likely you want M=0 memory
here ("non-memory memory", "not well-behaved memory", MMIO often).
Regular kernel memory vmapped with pgprot_noncached().
So, I=1 (and G=1). Caching inhibited and guarded. But M=1 (memory
coherence required) as with any other real memory :-)
quoted
quoted
If memset() is expected to be used with M=0, you cannot do any serious
optimisations to it at all. If memset() is expected to be used with I=1
it should use a separate code path for it, probably the caller should
make the distinction.
DMA coherent memory which uses uncached memory for platforms that
do not provide hardware dma coherence can end up just about anywhere
in the kernel. We could use special routines for a few places in
the DMA subsystem, but there might be plenty of others.
Yeah. It will just be plenty slow, as we see here, that's what the
warning is for; but it works just fine :-)
The memset() code itself could chech for the storage attributes, but
that is probably more expensive than just assuming the happy case.
Maybe someone could try it out though!
Hmm, Ok! For what's worth I can at least test memset with dcbz+trap and
what it was in 2015, without dcbz in the code path. How about that?
I figured out of all the offenders (ethernet, crypto and sata).
The sata/hard drive would be the most sensitive device to measure any
performance difference. the MyBook Live already had an harddrive
(Seagate ST380815AS (very old)) installed... so I went with that.
I test with OpenWrt, since it has a fully working PowerPC images for
the device, I can use initramfs (so HDD/SDD is idle) and provides a
very bare minimum the hdparm -t "benchmark".
(hdparm -t ... just reads for three seconds and tells you how much it read).
the unmodified 6.6.47 kernel scored:
| Timing buffered disk reads: 220 MB in 3.02 seconds = 72.93 MB/sec
| Timing buffered disk reads: 222 MB in 3.02 seconds = 73.50 MB/sec
| Timing buffered disk reads: 216 MB in 3.00 seconds = 71.94 MB/sec
from what I can tell, each hdparm -t /dev/sda causes ~77000 fix_alignment traps.
(/sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size says it's 32 and
type is obviously "Data". If I'm not mistaken this means ~2400KiB of emulated
dcbz by the trap.)
For the test, I added the "old" memset from
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/lib/copy_32.S?id=df087e450d7ddc0b15bd8824206d964720b4f5e4#n120>
and replaced 6.6.47's memset in dma_pool_alloc() with it
<https://elixir.bootlin.com/linux/v6.6.47/source/mm/dmapool.c#L435>
now no WARNINGS are triggered and hdparm -t /dev/sda produces:
| Timing buffered disk reads: 220 MB in 3.00 seconds = 73.32 MB/sec
| Timing buffered disk reads: 218 MB in 3.02 seconds = 72.28 MB/sec
| Timing buffered disk reads: 224 MB in 3.03 seconds = 74.02 MB/sec
virtually no benefit?! Well, the HDD could be too slow. Let's try an old SSD:
Samsung 840 Evo 120 GB. This one manages to read 1276 MB in 3.06 seconds = ~416 MB/sec
in the same hdparm -t test on a reasonably modern PC when connected via a
usb3<->sata adapter.
unmodified 6.6.47 kernel:
| Timing buffered disk reads: 356 MB in 3.00 seconds = 118.61 MB/sec
| Timing buffered disk reads: 358 MB in 3.01 seconds = 119.12 MB/sec
| Timing buffered disk reads: 358 MB in 3.01 seconds = 119.03 MB/sec
modified 6.6.47 kernel:
| Timing buffered disk reads: 380 MB in 3.01 seconds = 126.30 MB/sec
| Timing buffered disk reads: 374 MB in 3.00 seconds = 124.61 MB/sec
| Timing buffered disk reads: 382 MB in 3.02 seconds = 126.62 MB/sec
Ok! There's something there. ~4%.
Cheers,
Christian
Le 23/08/2024 à 21:19, Segher Boessenkool a écrit :
***ATTENTION, Sopra Steria Group cannot confirm the identity of this email sender (SPF record failure). This might be a fake email from an attacker, if you have any doubts report and delete the email.***
***ATTENTION, Sopra Steria Group ne peut pas confirmer l’identité de l’émetteur de ce message (SPF record failure). Il pourrait s’agir d’un faux message, à détruire si vous avez un doute ***
Hi!
On Fri, Aug 23, 2024 at 03:54:59PM +0200, Christoph Hellwig wrote:
quoted
On Fri, Aug 23, 2024 at 08:06:00AM -0500, Segher Boessenkool wrote:
quoted
What does "uncached memory" even mean here? Literally it would be
I=1 memory (uncachEABLE memory), but more likely you want M=0 memory
here ("non-memory memory", "not well-behaved memory", MMIO often).
Regular kernel memory vmapped with pgprot_noncached().
So, I=1 (and G=1). Caching inhibited and guarded. But M=1 (memory
coherence required) as with any other real memory :-)
quoted
quoted
If memset() is expected to be used with M=0, you cannot do any serious
optimisations to it at all. If memset() is expected to be used with I=1
it should use a separate code path for it, probably the caller should
make the distinction.
DMA coherent memory which uses uncached memory for platforms that
do not provide hardware dma coherence can end up just about anywhere
in the kernel. We could use special routines for a few places in
the DMA subsystem, but there might be plenty of others.
Yeah. It will just be plenty slow, as we see here, that's what the
warning is for; but it works just fine :-)
The memset() code itself could chech for the storage attributes, but
that is probably more expensive than just assuming the happy case.
Maybe someone could try it out though!
But is it only memset() the problem ?
dcbz instruction is also used in:
- memcpy()
- csum_partial_copy_generic()
- clear_page()
- copy_page()
- clear_user()
- copy_to_user()
- copy_from_user()
Are these functions also used on DMA coherent memory ?
Christophe
On Sat, Aug 24, 2024 at 09:01:33AM +0000, LEROY Christophe wrote:
Le 23/08/2024 à 21:19, Segher Boessenkool a écrit :
quoted
The memset() code itself could chech for the storage attributes, but
that is probably more expensive than just assuming the happy case.
Maybe someone could try it out though!
But is it only memset() the problem ?
dcbz instruction is also used in:
- memcpy()
- csum_partial_copy_generic()
- clear_page()
- copy_page()
- clear_user()
- copy_to_user()
- copy_from_user()
That is just a handful of functions. Not sure about the _user things,
and the _page things for that matter, but the rest is certainly
measurable in real-life conditions. So if we can avoid the problems
completely, and cheaply, we probably should.
I'm not so sure about the cheaply though :-/
Are these functions also used on DMA coherent memory ?
Most won't show up high on most profiles, heh. Which you already
can see from the problem not being attacked yet: if it was so obviously
a problem, some people would have wanted to do something about it :-)
Segher
From: Christoph Hellwig <hch@lst.de> Date: 2024-08-27 07:29:50
On Sat, Aug 24, 2024 at 12:17:57PM -0500, Segher Boessenkool wrote:
quoted
Are these functions also used on DMA coherent memory ?
Most won't show up high on most profiles, heh. Which you already
can see from the problem not being attacked yet: if it was so obviously
a problem, some people would have wanted to do something about it :-)
Most drivers try to avoid coherent allocations in the fast path if
they can. Another good option for Christians problem would be
to switch the the dmaengine driver to use dma_alloc_pages - it doesn't
actually need uncached memory as far as I can, dma_alloc_coherent is
just the only API we had to allocate guaranteed DMAable memory for most
of Linux's existence.