Re: m68k boot failure in -next bisected to 'xarray: Replace exceptional entries'

[Guenter Roeck <linux@roeck-us.net>]

On 06/23/2018 12:46 AM, Matthew Wilcox wrote:

>>
>> Here is the culprit:
>>
>> genl_register_family(0x36dd7a) registering VFS_DQUOT
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at lib/idr.c:42 idr_alloc_u32+0x44/0xe8
>>
>> It may be odd that fs/quota/netlink.c:quota_genl_family is not word
>> aligned, but on the other side I don't think there is a rule that
>> the function parameter to genl_register_family() - or the second
>> parameter of idr_alloc() - must be word aligned. Am I missing
>> something ? After all, it could be a pointer to the nth element
>> of a string, or the caller could on purpose allocate IDRs for
>> (ptr), (ptr + 1), and so on.
> 
> There actually is a rule that pointers passed to the IDR be aligned.
> It might not be written down anywhere ;-)  And I'm quite happy to lift
> that restriction; after all I don't want to force everybody to decorate
> definitions with __aligned(4).
> 

I am not sure if that is really correct, or at least I was unable to
find such a restriction documented or even mentioned anywhere. It is
true for radix trees, but even there the restriction used to be "even".
This was changed to "word aligned" with commit 3bcadd6fa6c4f, and the
offending commit here moves the "INTERNAL" bit from position 0 to 1.
This is quite a subtle change that was introduced over time. I'd be
not surprised if there are other more severe problems lurking in
the radix tree code and/or its users because of that.

Either case, I think the check for radix_tree_is_internal_node() in
idr_alloc_u32() is wrong. If anything, it should be radix_tree_exception().
If that was the case, the problem (and maybe other similar problems)
would have been found with commit 3bcadd6fa6c4f, not only now.

Guenter