Re: [PATCH] mapletree-vs-khugepaged

[Sven Schnelle <svens@linux.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2874 bytes --]

Hi Liam,

Liam Howlett <liam.howlett@oracle.com> writes:

> * Sven Schnelle <svens@linux.ibm.com> [220515 16:02]:
>
> I tried the above on my qemu s390 with kernel 5.18.0-rc6-next-20220513,
> but it runs without issue, return code is 0.  Is there something the VM
> needs to have for this to trigger?

A coworker said the same. Reason for this seems to be that i've run the
code in a unittest environment which seems to make a difference. When
compiling the code above with gcc on my system it also doesn't crash.
So i have to figure out what makes this unittest binary special.

>> I've added a few debug statements to the maple tree code:
>> 
>> [   27.769641] mas_next_entry: offset=14
>> [   27.769642] mas_next_nentry: entry = 0e00000000000000, slots=0000000090249f80, mas->offset=15 count=14
>
> Where exactly are you printing this?

I added a lot of debug statements to the code trying to understand
it. I'll attach it to this mail.

>> 
>> I see in mas_next_nentry() that there's a while that iterates over the
>> (used?) slots until count is reached.`
>
> Yes, mas_next_nentry() looks for the next non-null entry in the current
> node.
>
>>After that loop mas_next_entry()
>> just picks the next (unused?) entry, which is slot 15 in that case.
>
> mas_next_entry() returns the next non-null entry.  If there isn't one
> returned by mas_next_nentry(), then it will advance to the next node by
> calling mas_next_node().  There are checks in there for detecting dead
> nodes for RCU use and limit checking as well.
>
>> 
>> What i noticed while scanning over include/linux/maple_tree.h is:
>> 
>> struct maple_range_64 {
>> 	struct maple_pnode *parent;
>> 	unsigned long pivot[MAPLE_RANGE64_SLOTS - 1];
>> 	union {
>> 		void __rcu *slot[MAPLE_RANGE64_SLOTS];
>> 		struct {
>> 		void __rcu *pad[MAPLE_RANGE64_SLOTS - 1];
>> 		struct maple_metadata meta;
>>         	};
>> 	};
>> };
>> 
>> and struct maple_metadata is:
>> 
>> struct maple_metadata {
>> 	unsigned char end;
>> 	unsigned char gap;
>> };
>> 
>> If i swap the gap and end members 0x0e00000000000000 becomes
>> 0x000e000000000000. And 0xe matches our msa->offset 14 above.
>> So it looks like mas_next() in mmap_region returns the meta
>> data for the node.
>
> If this is the case, then I think any task that has more than 14 VMAs
> would have issues.  I also use mas_next_entry() in mas_find() which is
> used for the mas_for_each() macro/iterator.  Can you please enable
> CONFIG_DEBUG_VM_MAPLE_TREE ?  mmap.c tests the tree after pretty much
> any change and will dump useful information if there is an issue -
> including the entire tree. See validate_mm_mt() for details.
>
> You can find CONFIG_DEBUG_VM_MAPLE_TREE in the config:
> kernel hacking -> Memory debugging -> Debug VM -> Debug VM maple trees

I have both DEBUG_MAPPLE_TREE and DEBUG_VM_MAPLE_TREE enabled, but don't
see anything printed.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mapple-debug.diff --]
[-- Type: text/x-diff, Size: 172 bytes --]

 lib/maple_tree.c | 37 +++++++++++++++++++++++++++++++++++--
 mm/mmap.c        | 36 +++++++++++++++++++++++++++++-------
 2 files changed, 64 insertions(+), 9 deletions(-)