Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 15 Sep 2017 16:38:28 -0700
From:      John Baldwin <jhb@freebsd.org>
To:        "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>
Subject:   UMA and zone alignment
Message-ID:  <10066620.9raPCKhsf8@ralph.baldwin.cx>

next in thread | raw e-mail | index | archive | help
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?

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?10066620.9raPCKhsf8>