Re: [PATCH -tip v3 0/4] x86,mm/pat: Move towards using generic interval tree

From: Ingo Molnar
Date: Thu Nov 21 2019 - 00:57:32 EST



* Davidlohr Bueso <dave@xxxxxxxxxxxx> wrote:

> Changes from v2:
> - Removed unnecessary goto error path in patch 1, per tglx.
> - Added the corresponding Makefile change for patch 4, per mingo.
> - Added tglx's review tags.
>
> Changes from v1[0]:
> - Got rid of more code in patch 1 by using the end - 1 for closed
> intervals, instead of keeping the overlap-check.
>
> - added an additional cleanup patch.
>
> Hi,
>
> I'm sending this series again in this format as the interval tree
> node conversion will, at a minimum, take longer than hoped for
> (ie: Jason still removing interval tree users for the mmu_notifier
> rework[1]). There is also a chance this will never see be done.
>
> As such, I'm resending this series (where patch 1 is the only
> interesting one and which Ingo acked previously, with the exception
> that the nodes remain fully closed). In the future, it would be
> trivial to port pat tree to semi open nodes, but for now think that
> it makes sense to just get the pat changes in.
>
> Please consider for v5.5. Thanks!
>
> [0] https://lore.kernel.org/lkml/20190813224620.31005-1-dave@xxxxxxxxxxxx/
> [1] https://marc.info/?l=linux-mm&m=157116340411211
>
> Davidlohr Bueso (4):
> x86/mm, pat: Convert pat tree to generic interval tree
> x86/mm, pat: Cleanup some of the local memtype_rb_* calls
> x86/mm, pat: Drop rbt prefix from external memtype calls
> x86/mm, pat: Rename pat_rbtree.c to pat_interval.c
>
> arch/x86/mm/Makefile | 2 +-
> arch/x86/mm/pat.c | 8 +-
> arch/x86/mm/pat_internal.h | 20 ++--
> arch/x86/mm/pat_interval.c | 185 +++++++++++++++++++++++++++++++
> arch/x86/mm/pat_rbtree.c | 268 ---------------------------------------------
> 5 files changed, 200 insertions(+), 283 deletions(-)
> create mode 100644 arch/x86/mm/pat_interval.c
> delete mode 100644 arch/x86/mm/pat_rbtree.c

Thanks Davidlohr - this is a really nice cleanup of the logic and of the
tree data structure, and I've applied your earlier series to
tip:WIP.x86/mm already, with a few more work-in-progress patches from me
on top that tidy up this area of the code.

In particular I've done a bunch of changes to improve the hackability of
all pat/memtype/set_memory facilities, we've now got <asm/memtype.h>,
memtype.[ch], memtype_interval.c and set_memory.c in arch/x86/mm/pat/:

dagon:~/tip> ls -l arch/x86/mm/pat/
total 112
-rw-r--r-- 1 mingo mingo 5782 Nov 21 06:41 cpa-test.c
-rw-r--r-- 1 mingo mingo 117 Nov 21 06:41 Makefile
-rw-r--r-- 1 mingo mingo 32026 Nov 21 06:41 memtype.c
-rw-r--r-- 1 mingo mingo 1470 Nov 21 06:41 memtype.h
-rw-r--r-- 1 mingo mingo 5003 Nov 21 06:41 memtype_interval.c
-rw-r--r-- 1 mingo mingo 56668 Nov 21 06:41 set_memory.c

( Note: cpa-test.c should probable be renamed to set_memory_test.c, with
a few explicit set_memory() API tests added as well, not just the
internal change_page_attribute() tests. Will do this later. )

The memtype APIs are (rightside column):

reserve_memtype() => memtype_reserve()
free_memtype() => memtype_free()
kernel_map_sync_memtype() => memtype_kernel_map_sync()
io_reserve_memtype() => memtype_reserve_io()
io_free_memtype() => memtype_free_io()

memtype_check_insert() => memtype_check_insert()
memtype_erase() => memtype_erase()
memtype_lookup() => memtype_lookup()
memtype_copy_nth_element() => memtype_copy_nth_element()

But there's a lot more changes:

218bf1a8c73b: x86/mm/pat: Convert the PAT tree to a generic interval tree
3309be371c20: x86/mm/pat: Clean up some of the local memtype_rb_*() calls
b40805c214c5: x86/mm/pat: Drop the rbt_ prefix from external memtype function names
ee4e7b04b718: x86/mm/pat: Rename pat_rbtree.c to pat_interval.c

8afed68b3426: x86/mm/pat: Update the comments in pat.c and pat_interval.c and refresh the code a bit
bc8a3eed1241: x86/mm/pat: Disambiguate PAT-disabled boot messages
10ffd914266a: x86/mm/pat: Create fixed width output in /sys/kernel/debug/x86/pat_memtype_list, similar to the E820 debug printouts
37dfd5d60000: x86/mm/pat: Simplify the free_memtype() control flow
b686cd38771d: x86/mm/pat: Harmonize 'struct memtype *' local variable and function parameter use
7fa6ebcdfb73: x86/mm/pat: Clean up PAT initialization flags
a224f826be60: x86/mm/pat: Move the memtype related files to arch/x86/mm/pat/
acb2f580640a: x86/mm/pat: Standardize on memtype_*() prefix for APIs
393cac16e6b7: x86/mm/pat: Rename <asm/pat.h> => <asm/memtype.h>
22a6d30c44c7: x86/mm/pat: Clean up <asm/memtype.h> externs
56ca0be07c28: x86/mm/pat: Fix typo in the Kconfig help text
07fbea7b3be2: x86/mm: Tabulate the page table encoding definitions

I'll send them out separately as well once completed, but wanted to give
you a heads-up. My patches are in WIP state because neither the
changelogs nor the split-up is necessarily final.

These can all be accessed and followed under:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/mm

What do you think about these changes? Anything else you'd like to see
happen?

In terms of upstreaming plans, the 4 commits from you I grouped
separately definitely look like v5.5 material to me - will merge them
into tip:x86/mm later today.

Thanks,

Ingo