Skip to contentSkip to conversation

slab debug vs. L1 alignement

BUG Manfred Spraul 3 participants 9 messages Aug 15, 2003 – Aug 16, 2003

lore
Share & tools
lore
#0 lore raw
Quoted 4 lines · Ben

Currently, when enabling slab debugging, we lose the property of having the objects aligned on a cache line size.

Correct. Cache line alignment is advisory. Slab debugging is not the only case that violates the alignment, for example 32-byte allocations are not padded to the 128 byte cache line size of the Pentium 4 cpus. I really doubt we want that.

Have you looked at pci_pool_{create,alloc,free,destroy}? The functions were specifically written to provide aligned buffers for DMA operations. Perhaps SCSI should use them?

--
Manfred

#1 lore raw
Quoted 10 lines · Manfred Spraul
Quoted 4 lines · Ben

Currently, when enabling slab debugging, we lose the property of having the objects aligned on a cache line size.

Correct. Cache line alignment is advisory. Slab debugging is not the only case that violates the alignment, for example 32-byte allocations are not padded to the 128 byte cache line size of the Pentium 4 cpus. I really doubt we want that.

Yes, I understand that, but that is wrong for GFP_DMA imho. Also, SLAB_MUST_HWCACHE_ALIGN just disables redzoning, which is not smart, I'd rather allocate more and keep both redzoning and cache alignement, that would help catch some of those subtle problems when a chip DMA engine plays funny tricks

Quoted 3 lines

Have you looked at pci_pool_{create,alloc,free,destroy}? The functions were specifically written to provide aligned buffers for DMA operations. Perhaps SCSI should use them?

Of course it should, but it doesn't yet. And other drivers haven't been ported yet neither though that is slowly happening. Still, I think the point that a GFP_DMA should be cache aligned still stands, don't you think ?

Ben.

#2 lore raw
Quoted 13 lines · Benjamin Herrenschmidt
Quoted 10 lines · Manfred Spraul
Quoted 6 lines · Ben

Currently, when enabling slab debugging, we lose the property of having the objects aligned on a cache line size.

Quoted 9 lines
Quoted 3 lines

Correct. Cache line alignment is advisory. Slab debugging is not the only case that violates the alignment, for example 32-byte allocations are not padded to the 128 byte cache line size of the Pentium 4 cpus. I really doubt we want that.

Yes, I understand that, but that is wrong for GFP_DMA imho. Also, SLAB_MUST_HWCACHE_ALIGN just disables redzoning, which is not smart, I'd rather allocate more and keep both redzoning and cache alignement,
that would help catch some of those subtle problems when a chip DMA
engine plays funny tricks.

I don't want to upgrade SLAB_HWCACHE_ALIGN to SLAB_MUST_HWCACHE_ALIGN depending on GFP_DMA: IIRC one arch (ppc64?) marks everything as GFP_DMA, because all memory is DMA capable.

Which arch do you use? Perhaps alignment could be added for broken archs.

Actually I think you should fix your arch, perhaps by double buffering in pci_map_ if the input pointers are not aligned. What if someone uses O_DIRECT with an unaligned pointer?

--
Manfred

#3 lore raw
Quoted 10 lines
Quoted 7 lines

Yes, I understand that, but that is wrong for GFP_DMA imho. Also, SLAB_MUST_HWCACHE_ALIGN just disables redzoning, which is not smart, I'd rather allocate more and keep both redzoning and cache alignement, that would help catch some of those subtle problems when a chip DMA engine plays funny tricks.

I don't want to upgrade SLAB_HWCACHE_ALIGN to SLAB_MUST_HWCACHE_ALIGN depending on GFP_DMA: IIRC one arch (ppc64?) marks everything as GFP_DMA, because all memory is DMA capable.

Same for ppc32. Anyway, I don't like MUST_HWCACHE_ALIGN because it just disables redzoning, I'd rather allocate more and do both redzoning and cache alignement.

In fact, I think cache alignement should be a property of all kmalloc calls for any size > L1 cache line size for obvious perfs reasons, and redzoning shouldn't change such a property...

Quoted 5 lines

Which arch do you use? Perhaps alignment could be added for broken archs.

Actually I think you should fix your arch, perhaps by double buffering in pci_map_ if the input pointers are not aligned. What if someone uses O_DIRECT with an unaligned pointer?

ppc32 "normally" is cache coherent, that is with high end or desktop CPUs, embedded CPUs aren't. Some archs like ARM arent't. But in this case, the problem is related to a broken SCSI controller on the motherboard which will cause corruption on non-aligned buffers (it basically always issues memory write and invalidate cycles).

Anyway, I _still_ think it's stupid to return non-aligned buffers, both for performances, and because that prevents from dealing with such cases, typically the SCSI layer assumes alignement here among others...

Regarding O_DIRECT with an unaligned pointer, I haven't looked at this case yet, I suppose it will be broken in a whole lot of cases.

Ben.

#4 lore raw
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 7 lines
Quoted 7 lines

Yes, I understand that, but that is wrong for GFP_DMA imho. Also, SLAB_MUST_HWCACHE_ALIGN just disables redzoning, which is not smart, I'd rather allocate more and keep both redzoning and cache alignement,
that would help catch some of those subtle problems when a chip DMA
engine plays funny tricks.

Quoted 9 lines
Quoted 3 lines

I don't want to upgrade SLAB_HWCACHE_ALIGN to SLAB_MUST_HWCACHE_ALIGN depending on GFP_DMA: IIRC one arch (ppc64?) marks everything as GFP_DMA, because all memory is DMA capable.

Same for ppc32. Anyway, I don't like MUST_HWCACHE_ALIGN because it just disables redzoning, I'd rather allocate more and do both redzoning and cache alignement.

I have a patch that creates helper functions that make that simple. The patch is stuck right now, because it exposes a bug in the i386 debug register handling. I'll add it redzoning with MUST_HWCACHE_ALIGN after that one is in.

Quoted 5 lines

Anyway, I _still_ think it's stupid to return non-aligned buffers, both for performances, and because that prevents from dealing with such cases, typically the SCSI layer assumes alignement here among others...

I don't care about performance with slab debugging on. kmalloc(4096,) usually takes ~40 cpu ticks on i386. With debugging on, it includes a memset and an open coded memcmp - my guess is a few thousand cpu ticks, and that's intentional. Do not enable it on production systems.

Quoted 4 lines

Regarding O_DIRECT with an unaligned pointer, I haven't looked at this case yet, I suppose it will be broken in a whole lot of cases.

Hmm. That means slab debugging did it's job: The driver contains the wrong assumption that all pointers are cache line aligned. Without slab debugging, this would result in rare data corruptions in O_DIRECT apps. With slab debugging on, it's exposed immediately.

--
Manfred

#5 lore raw
Quoted 6 lines
Quoted 2 lines

Hmm. That means slab debugging did it's job: The driver contains the wrong assumption that all pointers are cache line aligned. Without slab debugging, this would result in rare data corruptions in O_DIRECT apps. With slab debugging on, it's exposed immediately.

As we discussed on IRC, I think SCSI host drivers (at least) can assume beeing passed aligned buffers, if somebody don't agree, please speak up ;) Christoph said that O_DIRECT has strict alignement rules too.

Ben.

#6 lore raw
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 6 lines
Quoted 2 lines

Hmm. That means slab debugging did it's job: The driver contains the wrong assumption that all pointers are cache line aligned. Without slab debugging, this would result in rare data corruptions in O_DIRECT apps. With slab debugging on, it's exposed immediately.

As we discussed on IRC, I think SCSI host drivers (at least) can assume beeing passed aligned buffers, if somebody don't agree, please speak up ;) Christoph said that O_DIRECT has strict alignement rules too.

A character device (like st) doing direct i/o from user buffer to/from a SCSI device does not currently have any alignment restrictions. I think restricted alignment can't be required from a user of an ordinary character device. This must then be handled by the driver. The solution is to use bounce buffers in the driver if the alignment does not meet the lower level requirements. This leads to surprises with performance if the user buffer alignment does not satisfy the requirements (e.g., malloc() may or may not return properly aligned blocks). These surprises should be avoided as far as the hardware allows.

If an architecture has restrictions, they must, of course, be taken into account. However, this should not punish architectures that don't have the restrictions. Specifying that DMA buffers must be cache-line aligned would be too strict. A separate alignment constraint for DMA in general and for a device in specific would be a better alternative (a device may have tighter restrictions than an architecture). The same applies to buffer sizes. This would mean adding two more masks for each device (like the current DMA address mask for a device).

Signature · 1 line
Kai
#7 lore raw
Quoted 10 lines

->
A character device (like st) doing direct i/o from user buffer to/from a SCSI device does not currently have any alignment restrictions. I think restricted alignment can't be required from a user of an ordinary character device. This must then be handled by the driver. The solution is to use bounce buffers in the driver if the alignment does not meet the lower level requirements. This leads to surprises with performance if the user buffer alignment does not satisfy the requirements (e.g., malloc() may or may not return properly aligned blocks). These surprises should be avoided as far as the hardware allows.

THe low level driver can't do the bounce buffer thing, it has to be done at higher layers.

Quoted 8 lines

If an architecture has restrictions, they must, of course, be taken into account. However, this should not punish architectures that don't have the restrictions. Specifying that DMA buffers must be cache-line aligned would be too strict. A separate alignment constraint for DMA in general and for a device in specific would be a better alternative (a device may have tighter restrictions than an architecture). The same applies to buffer sizes. This would mean adding two more masks for each device (like the current DMA address mask for a device).

That won't help for buffers coming from higher layers that don't know the device they'll end up to

Ben.

#8 lore raw

On Sat, 16 Aug 2003, Benjamin Herrenschmidt wrote:

...

Quoted 3 lines

THe low level driver can't do the bounce buffer thing, it has to be done at higher layers.

Agreed. A high-level driver must be able to handle this anyway because of other constraints. The point is that this costs cpu cycles and bouncing should be avoided if possible.

Quoted 2 lines
Quoted 2 lines

If an architecture has restrictions, they must, of course, be taken into account. However, this should not punish architectures that don't have the

...

Quoted 6 lines
Quoted 2 lines

sizes. This would mean adding two more masks for each device (like the current DMA address mask for a device).

That won't help for buffers coming from higher layers that don't know the device they'll end up to

Agreed. This just enables the high-level driver to avoid bouncing. For instance, in a P4 system it is usually not necessary to require 128 byte alignment for DMA.

Signature · 1 line
Kai