Message-ID: <20191121055724.GA38182@gmail.com>
Date:   Thu, 21 Nov 2019 06:57:24 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Davidlohr Bueso <dave@...olabs.net>
Cc:     tglx@...utronix.de, bp@...en8.de, peterz@...radead.org,
        x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -tip v3 0/4] x86,mm/pat: Move towards using generic
 interval tree


* Davidlohr Bueso <dave@...olabs.net> 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@stgolabs.net/
> [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

Powered by blists - more mailing lists