UMA and zone alignment

[John Baldwin]

I ran into an odd panic recently while running an MIPS n32 kernel under qemu
and tried to use NFS. The panic was due to an unaligned 64-bit store here:

(gdb) l *cache_enter_time+0xe8
0x80359414 is in cache_enter_time (/usr/home/john/work/git/mips_xbuild/sys/kern/vfs_cache.c:1621).
1616 if (vp == NULL)
1617 ncp->nc_flag |= NCF_NEGATIVE;
1618 ncp->nc_dvp = dvp;
1619 if (tsp != NULL) {
1620 n3 = (struct namecache_ts *)ncp;
1621 n3->nc_time = *tsp;

The reason the store was unaligned is that the UMA zone that stores the
namecache structures has an alignment of UMA_ALIGN_PTR. However,
struct namecache_ts stores a time_t, so on 32-bit platforms with 64-bit
time_t (such as 32-bit mips, powerpc, arm) UMA_ALIGN_PTR only requests 4
byte alignment. For most 32-bit architectures this doesn't really matter
as they only use 32-bit loads and stores and have to load a 64-bit time_t
as two operations. However, MIPS n32 uses 32-bit pointers with 64-bit
registers and thus uses 64-bit loads and stores. As a result, when UMA
allocated a structure that was 32-bit (but not 64-bit) aligned, the store
to save the time_t in 'nc_time' faulted:

db> tr
Tracing pid 987 tid 100057 td 0x80a5f000
cache_enter_time+0xe8 (?,?,?,?) ra ffffffff801f41dc sp ffffffffe5943470 sz 144
nfscl_doiods+0x516c (ffffffff80945420,?,?,?) ra 0 sp ffffffffe5943500 sz 0
db> x/i $pc
cache_enter_time+0xe8: sd v1,40(s4)
db> p/x $s4
80aaa984

I pondered hacking UMA_ALIGN_PTR on N32 to be sizeof(uint64_t) instead of
sizeof(void *), but instead I would rather we stop trying to guess what the
alignment of a given structure should be and let the compiler do that via
_Alignof(). We have a fallback __alignof() for pre-C11 environments, so I
believe we are fine to use this always (well, if have a pre-GCC 2.95 compiler
you can't use this, but if you have that you can't compile the kernel anyway).

To that end I added a UMA_ALIGNOF() which wraps _Alignof():

https://github.com/freebsd/freebsd/commit/a9d94dba3d78cfce330175a44445efec8531286e

and then used that to initialize the zones in vfs_cache.c:

https://github.com/freebsd/freebsd/commit/64271272bb30cc3b00ff22c1c44ed20582bea8b9

and now I can use NFS with a MIPS n32 kernels without insta-panics.

I think I'd like to add UMA_ALIGNOF() and encourage UMA zones to use
UMA_ALIGNOF(type) instead of UMA_ALIGN_PTR, etc. going forward. This would
mean UMA would then honor 'struct foo { ... } __aligned(XX)', etc. What do
you all think?

[Ed Schouten]

Post by John Baldwin
I think I'd like to add UMA_ALIGNOF() and encourage UMA zones to use
UMA_ALIGNOF(type) instead of UMA_ALIGN_PTR, etc. going forward. This would
mean UMA would then honor 'struct foo { ... } __aligned(XX)', etc. What do
you all think?

Awesome!

Random remark: it looks like uma_zcreate() already takes the size of
the objects to allocate without any wrapping; it's just sizeof(...).
Wouldn't we be able to do this for alignment as well?

More specifically, if all of the UMA_ALIGN_* definitions were
incremented by one and uma_zcreate() decrements it, we could eliminate
the UMA_ALIGNOF() macro entirely and simply use _Alignof() at the call
site directly.

If people don't like _Alignof()'s spelling, we could always consider
making something like <stdalign.h> available in kernel space.

[John Baldwin]

Post by Ed Schouten

Post by John Baldwin
I think I'd like to add UMA_ALIGNOF() and encourage UMA zones to use
UMA_ALIGNOF(type) instead of UMA_ALIGN_PTR, etc. going forward. This would
mean UMA would then honor 'struct foo { ... } __aligned(XX)', etc. What do
you all think?

Awesome!
Random remark: it looks like uma_zcreate() already takes the size of
the objects to allocate without any wrapping; it's just sizeof(...).
Wouldn't we be able to do this for alignment as well?
More specifically, if all of the UMA_ALIGN_* definitions were
incremented by one and uma_zcreate() decrements it, we could eliminate
the UMA_ALIGNOF() macro entirely and simply use _Alignof() at the call
site directly.

Fixing this part of the UMA API is also on my todo list, but I haven't
gotten to it yet (and this is quicker to fix).