Re: [PATCH V2] mm: Introduce GFP_PGTABLE
Matthew Wilcox writes: > On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: >> On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: >> > All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL | >> > __GFP_ZERO) and using it for allocating page table pages. This causes some >> > code duplication which can be easily avoided. GFP_KERNEL allocated and >> > cleared out pages (__GFP_ZERO) are required for page tables on any given >> > architecture. This creates a new generic GFP flag flag which can be used >> > for any page table page allocation. Does not cause any functional change. >> > >> > GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the >> > generic page tabe header just to prevent it's potential misuse as a general >> > allocation flag if included in include/linux/gfp.h. >> >> I haven't reviewed the patch yet but I am wondering whether this is >> really worth it without going all the way down to unify the common code >> and remove much more code duplication. Or is this not possible for some >> reason? > > Exactly what I suggested doing in response to v1. > > Also, the approach taken here is crazy. x86 has a feature that no other > architecture has bothered to implement yet -- accounting page tables > to the process. powerpc does __GFP_ACCOUNT for user page tables too. Shamelessly cribbed from the x86 version of course :) cheers
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed, Jan 16, 2019 at 02:47:16PM +0100, Christophe Leroy wrote: > Le 16/01/2019 à 14:18, Matthew Wilcox a écrit : > > I disagree with your objective. Making more code common is a great idea, > > but this patch is too unambitious. We should be heading towards one or > > two page table allocation functions instead of having every architecture do > > its own thing. > > > > So start there. Move the x86 function into common code and convert one > > other architecture to use it too. > > Are we talking about pte_alloc_one_kernel() and pte_alloc_one() ? > > I'm not sure x86 function is the best common one, as it seems to allocate a > multiple of PAGE_SIZE only. And that's the common case. Most architectures use a single page for at least one level of the pte/pmd/pud/p4d/pgd hierarchy. Some use multiple pages and some use a fraction of a page. > Some arches like powerpc use pagetables which are smaller than a page, for > instance powerpc 8xx uses 4k pagetables even with 16k pages, which means a > single page can be used by 4 pagetables. Those can be added later. Note I said "one or two", and that's what I had in mind; I think we want one function that allocates just a page and another that allocates a page fragment. Then we can have a good discussion about what method we use; s390 and ppc use different techniques today and there's really no good reason for that.
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
Le 16/01/2019 à 14:18, Matthew Wilcox a écrit : On Wed, Jan 16, 2019 at 06:42:22PM +0530, Anshuman Khandual wrote: On 01/16/2019 06:00 PM, Matthew Wilcox wrote: On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL | __GFP_ZERO) and using it for allocating page table pages. This causes some code duplication which can be easily avoided. GFP_KERNEL allocated and cleared out pages (__GFP_ZERO) are required for page tables on any given architecture. This creates a new generic GFP flag flag which can be used for any page table page allocation. Does not cause any functional change. GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the generic page tabe header just to prevent it's potential misuse as a general allocation flag if included in include/linux/gfp.h. I haven't reviewed the patch yet but I am wondering whether this is really worth it without going all the way down to unify the common code and remove much more code duplication. Or is this not possible for some reason? Exactly what I suggested doing in response to v1. Also, the approach taken here is crazy. x86 has a feature that no other architecture has bothered to implement yet -- accounting page tables to the process. Yet instead of spreading that goodness to all other architectures, Anshuman has gone to more effort to avoid doing that. The basic objective for this patch is to create a common minimum allocation flag that can be used by architectures but that still allows archs to add on additional constraints if they see fit. This patch does not intend to change functionality for any arch. I disagree with your objective. Making more code common is a great idea, but this patch is too unambitious. We should be heading towards one or two page table allocation functions instead of having every architecture do its own thing. So start there. Move the x86 function into common code and convert one other architecture to use it too. Are we talking about pte_alloc_one_kernel() and pte_alloc_one() ? I'm not sure x86 function is the best common one, as it seems to allocate a multiple of PAGE_SIZE only. Some arches like powerpc use pagetables which are smaller than a page, for instance powerpc 8xx uses 4k pagetables even with 16k pages, which means a single page can be used by 4 pagetables. Therefore, I would suggest to start with powerpc functions. Christophe
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed 16-01-19 18:57:13, Anshuman Khandual wrote: > > > On 01/16/2019 06:14 PM, Michal Hocko wrote: > > On Wed 16-01-19 04:30:18, Matthew Wilcox wrote: > >> On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: > >>> On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: > All architectures have been defining their own PGALLOC_GFP as > (GFP_KERNEL | > __GFP_ZERO) and using it for allocating page table pages. This causes > some > code duplication which can be easily avoided. GFP_KERNEL allocated and > cleared out pages (__GFP_ZERO) are required for page tables on any given > architecture. This creates a new generic GFP flag flag which can be used > for any page table page allocation. Does not cause any functional change. > > GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is > the > generic page tabe header just to prevent it's potential misuse as a > general > allocation flag if included in include/linux/gfp.h. > >>> > >>> I haven't reviewed the patch yet but I am wondering whether this is > >>> really worth it without going all the way down to unify the common code > >>> and remove much more code duplication. Or is this not possible for some > >>> reason? > >> > >> Exactly what I suggested doing in response to v1. > >> > >> Also, the approach taken here is crazy. x86 has a feature that no other > >> architecture has bothered to implement yet -- accounting page tables > >> to the process. Yet instead of spreading that goodness to all other > >> architectures, Anshuman has gone to more effort to avoid doing that. > > > > Yes, I believe the only reason this is x86 only is that each arch would > > have to be tweaked separately. So a cleanup in _that_ regard would be > > helpful. There is no real reason to have ptes accounted only for x86. > > There might be some exceptions but well, our asm-generic allows to opt > > in for generic implementation or override it with a special one. The > > later should be an exception rather than the rule. > > Fair enough. So we seem to have agreement over __GFP_ACCOUNT for user page > tables but not for the kernel. But should we accommodate __GFP_RETRY_MAYFAIL > or drop them altogether (including multi order allocation requests) ? __GFP_RETRY_MAYFAIL is a leftover from __GFP_REPEAT times and from my recollection it was pointless there. Or do we have any pte allocations which are costly high order or that would rather fail than OOM kill and retry. Really there is no reason to preserve some code just because it is written that way. Anyway, as Mathew already pointed out. There is no real reason to convert all arches at once. If you carve out a common implementation and make it arch generic then you can go once arch at the time a reuse the generic implementation. If there are still some arches which differ only by __GFP_RETRY_MAYFAIL then talk to respective maintainers and ask them what is the underlying reason. See how that is a completely different and much more efficient way than trying to make GFP_PGTABLE special and keep the code around? -- Michal Hocko SUSE Labs
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed 16-01-19 05:18:27, Matthew Wilcox wrote: > On Wed, Jan 16, 2019 at 06:42:22PM +0530, Anshuman Khandual wrote: > > On 01/16/2019 06:00 PM, Matthew Wilcox wrote: > > > On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: > > >> On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: > > >>> All architectures have been defining their own PGALLOC_GFP as > > >>> (GFP_KERNEL | > > >>> __GFP_ZERO) and using it for allocating page table pages. This causes > > >>> some > > >>> code duplication which can be easily avoided. GFP_KERNEL allocated and > > >>> cleared out pages (__GFP_ZERO) are required for page tables on any given > > >>> architecture. This creates a new generic GFP flag flag which can be used > > >>> for any page table page allocation. Does not cause any functional > > >>> change. > > >>> > > >>> GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is > > >>> the > > >>> generic page tabe header just to prevent it's potential misuse as a > > >>> general > > >>> allocation flag if included in include/linux/gfp.h. > > >> > > >> I haven't reviewed the patch yet but I am wondering whether this is > > >> really worth it without going all the way down to unify the common code > > >> and remove much more code duplication. Or is this not possible for some > > >> reason? > > > > > > Exactly what I suggested doing in response to v1. > > > > > > Also, the approach taken here is crazy. x86 has a feature that no other > > > architecture has bothered to implement yet -- accounting page tables > > > to the process. Yet instead of spreading that goodness to all other > > > architectures, Anshuman has gone to more effort to avoid doing that. > > > > The basic objective for this patch is to create a common minimum allocation > > flag that can be used by architectures but that still allows archs to add > > on additional constraints if they see fit. This patch does not intend to > > change functionality for any arch. > > I disagree with your objective. Making more code common is a great idea, > but this patch is too unambitious. We should be heading towards one or > two page table allocation functions instead of having every architecture do > its own thing. > > So start there. Move the x86 function into common code and convert one > other architecture to use it too. Agreed! -- Michal Hocko SUSE Labs
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On 01/16/2019 06:14 PM, Michal Hocko wrote: > On Wed 16-01-19 04:30:18, Matthew Wilcox wrote: >> On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: >>> On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL | __GFP_ZERO) and using it for allocating page table pages. This causes some code duplication which can be easily avoided. GFP_KERNEL allocated and cleared out pages (__GFP_ZERO) are required for page tables on any given architecture. This creates a new generic GFP flag flag which can be used for any page table page allocation. Does not cause any functional change. GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the generic page tabe header just to prevent it's potential misuse as a general allocation flag if included in include/linux/gfp.h. >>> >>> I haven't reviewed the patch yet but I am wondering whether this is >>> really worth it without going all the way down to unify the common code >>> and remove much more code duplication. Or is this not possible for some >>> reason? >> >> Exactly what I suggested doing in response to v1. >> >> Also, the approach taken here is crazy. x86 has a feature that no other >> architecture has bothered to implement yet -- accounting page tables >> to the process. Yet instead of spreading that goodness to all other >> architectures, Anshuman has gone to more effort to avoid doing that. > > Yes, I believe the only reason this is x86 only is that each arch would > have to be tweaked separately. So a cleanup in _that_ regard would be > helpful. There is no real reason to have ptes accounted only for x86. > There might be some exceptions but well, our asm-generic allows to opt > in for generic implementation or override it with a special one. The > later should be an exception rather than the rule. Fair enough. So we seem to have agreement over __GFP_ACCOUNT for user page tables but not for the kernel. But should we accommodate __GFP_RETRY_MAYFAIL or drop them altogether (including multi order allocation requests) ?
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed, Jan 16, 2019 at 06:42:22PM +0530, Anshuman Khandual wrote: > On 01/16/2019 06:00 PM, Matthew Wilcox wrote: > > On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: > >> On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: > >>> All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL > >>> | > >>> __GFP_ZERO) and using it for allocating page table pages. This causes some > >>> code duplication which can be easily avoided. GFP_KERNEL allocated and > >>> cleared out pages (__GFP_ZERO) are required for page tables on any given > >>> architecture. This creates a new generic GFP flag flag which can be used > >>> for any page table page allocation. Does not cause any functional change. > >>> > >>> GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the > >>> generic page tabe header just to prevent it's potential misuse as a > >>> general > >>> allocation flag if included in include/linux/gfp.h. > >> > >> I haven't reviewed the patch yet but I am wondering whether this is > >> really worth it without going all the way down to unify the common code > >> and remove much more code duplication. Or is this not possible for some > >> reason? > > > > Exactly what I suggested doing in response to v1. > > > > Also, the approach taken here is crazy. x86 has a feature that no other > > architecture has bothered to implement yet -- accounting page tables > > to the process. Yet instead of spreading that goodness to all other > > architectures, Anshuman has gone to more effort to avoid doing that. > > The basic objective for this patch is to create a common minimum allocation > flag that can be used by architectures but that still allows archs to add > on additional constraints if they see fit. This patch does not intend to > change functionality for any arch. I disagree with your objective. Making more code common is a great idea, but this patch is too unambitious. We should be heading towards one or two page table allocation functions instead of having every architecture do its own thing. So start there. Move the x86 function into common code and convert one other architecture to use it too.
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On 01/16/2019 06:00 PM, Matthew Wilcox wrote: > On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: >> On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: >>> All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL | >>> __GFP_ZERO) and using it for allocating page table pages. This causes some >>> code duplication which can be easily avoided. GFP_KERNEL allocated and >>> cleared out pages (__GFP_ZERO) are required for page tables on any given >>> architecture. This creates a new generic GFP flag flag which can be used >>> for any page table page allocation. Does not cause any functional change. >>> >>> GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the >>> generic page tabe header just to prevent it's potential misuse as a general >>> allocation flag if included in include/linux/gfp.h. >> >> I haven't reviewed the patch yet but I am wondering whether this is >> really worth it without going all the way down to unify the common code >> and remove much more code duplication. Or is this not possible for some >> reason? > > Exactly what I suggested doing in response to v1. > > Also, the approach taken here is crazy. x86 has a feature that no other > architecture has bothered to implement yet -- accounting page tables > to the process. Yet instead of spreading that goodness to all other > architectures, Anshuman has gone to more effort to avoid doing that. > The basic objective for this patch is to create a common minimum allocation flag that can be used by architectures but that still allows archs to add on additional constraints if they see fit. This patch does not intend to change functionality for any arch. Yes. There is opportunity for further clean up and consolidation like the one you mentioned about accounting. Uses pages tables should have __GFP_ACCOUNT and kernel ones should not. IIUC unfortunately not all arch implement this right now. It is something which should not be arch specific. Accounting semantics should be common for all archs. A default function switching between GFP_PGTABLE for kernel and GFP_PGTABLE | __GFP_ACCOUNT looking into mm_struct can help here. Then there is __GFP_RETRY_MAYFAIL. Some archs use it for multi order page allocation but some use for a single page as well. If there is an agreement on __GFP_ACCOUNT and __GFP_RETRY_MAYFAIL we can clean this up further.
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On 01/16/2019 12:25 PM, Mike Rapoport wrote: > On Wed, Jan 16, 2019 at 11:51:32AM +0530, Anshuman Khandual wrote: >> All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL | >> __GFP_ZERO) and using it for allocating page table pages. This causes some >> code duplication which can be easily avoided. GFP_KERNEL allocated and >> cleared out pages (__GFP_ZERO) are required for page tables on any given >> architecture. This creates a new generic GFP flag flag which can be used >> for any page table page allocation. Does not cause any functional change. >> >> GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the >> generic page tabe header just to prevent it's potential misuse as a general >> allocation flag if included in include/linux/gfp.h. >> >> Signed-off-by: Anshuman Khandual >> --- >> Build tested on arm, arm64, powerpc, powerpc64le and x86. >> Boot tested on arm64 and x86. >> >> Changes in V2: >> >> - Moved GFP_PGTABLE into include/asm-generic/pgtable.h >> - On X86 added __GFP_ACCOUNT into GFP_PGTABLE at various places >> - Replaced possible flags on riscv and nds32 with GFP_PGTABLE >> >> Original V1: https://lkml.org/lkml/2019/1/12/54 >> >> arch/arm/include/asm/pgalloc.h | 8 +++- >> arch/arm/mm/mmu.c| 2 +- >> arch/arm64/include/asm/pgalloc.h | 9 - >> arch/arm64/mm/mmu.c | 2 +- >> arch/arm64/mm/pgd.c | 4 ++-- >> arch/nds32/include/asm/pgalloc.h | 3 +-- >> arch/powerpc/include/asm/book3s/64/pgalloc.h | 6 +++--- >> arch/powerpc/include/asm/pgalloc.h | 2 -- >> arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +- >> arch/powerpc/mm/pgtable-frag.c | 4 ++-- >> arch/riscv/include/asm/pgalloc.h | 8 +++- >> arch/sh/mm/pgtable.c | 6 ++ >> arch/unicore32/include/asm/pgalloc.h | 6 ++ >> arch/x86/kernel/espfix_64.c | 6 ++ >> arch/x86/mm/pgtable.c| 15 +++ >> include/asm-generic/pgtable.h| 2 ++ >> virt/kvm/arm/mmu.c | 2 +- >> 17 files changed, 37 insertions(+), 50 deletions(-) > I wonder, what about the other arches? Do they use different GFP flags? > Some of them as listed below use (GFP_KERNEL | __GFP_ZERO) which I will fix next time around. Some how was focused on removing PGALLOC_GFP that missed the other ones. arch/powerpc/include/asm/nohash/64/pgalloc.h arch/alpha/include/asm/pgalloc.h arch/alpha/mm/init.c arch/csky/include/asm/pgalloc.h arch/arc/include/asm/pgalloc.h But then there are those which use GFP_KERNEL alone without __GFP_ZERO like pmd_alloc_one() in arch/sparc/include/asm/pgalloc_64.h cannot be replaced with this patch as it does not intend to change functionality.
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed 16-01-19 04:30:18, Matthew Wilcox wrote: > On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: > > On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: > > > All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL > > > | > > > __GFP_ZERO) and using it for allocating page table pages. This causes some > > > code duplication which can be easily avoided. GFP_KERNEL allocated and > > > cleared out pages (__GFP_ZERO) are required for page tables on any given > > > architecture. This creates a new generic GFP flag flag which can be used > > > for any page table page allocation. Does not cause any functional change. > > > > > > GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the > > > generic page tabe header just to prevent it's potential misuse as a > > > general > > > allocation flag if included in include/linux/gfp.h. > > > > I haven't reviewed the patch yet but I am wondering whether this is > > really worth it without going all the way down to unify the common code > > and remove much more code duplication. Or is this not possible for some > > reason? > > Exactly what I suggested doing in response to v1. > > Also, the approach taken here is crazy. x86 has a feature that no other > architecture has bothered to implement yet -- accounting page tables > to the process. Yet instead of spreading that goodness to all other > architectures, Anshuman has gone to more effort to avoid doing that. Yes, I believe the only reason this is x86 only is that each arch would have to be tweaked separately. So a cleanup in _that_ regard would be helpful. There is no real reason to have ptes accounted only for x86. There might be some exceptions but well, our asm-generic allows to opt in for generic implementation or override it with a special one. The later should be an exception rather than the rule. -- Michal Hocko SUSE Labs
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On 01/16/2019 12:40 PM, Christophe Leroy wrote: > > > Le 16/01/2019 à 07:21, Anshuman Khandual a écrit : >> All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL | >> __GFP_ZERO) and using it for allocating page table pages. This causes some >> code duplication which can be easily avoided. GFP_KERNEL allocated and >> cleared out pages (__GFP_ZERO) are required for page tables on any given >> architecture. This creates a new generic GFP flag flag which can be used >> for any page table page allocation. Does not cause any functional change. >> >> GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the >> generic page tabe header just to prevent it's potential misuse as a general >> allocation flag if included in include/linux/gfp.h. >> >> Signed-off-by: Anshuman Khandual >> --- >> Build tested on arm, arm64, powerpc, powerpc64le and x86. >> Boot tested on arm64 and x86. >> >> Changes in V2: >> >> - Moved GFP_PGTABLE into include/asm-generic/pgtable.h >> - On X86 added __GFP_ACCOUNT into GFP_PGTABLE at various places >> - Replaced possible flags on riscv and nds32 with GFP_PGTABLE > > Could also replace the flags in arch/powerpc/include/asm/nohash/64/pgalloc.h > with GFP_PGTABLE in pte_alloc_one_kernel() and pte_alloc_one() Sure will do.
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed, Jan 16, 2019 at 04:30:18AM -0800, Matthew Wilcox wrote: > On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: > > On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: > > > All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL > > > | > > > __GFP_ZERO) and using it for allocating page table pages. This causes some > > > code duplication which can be easily avoided. GFP_KERNEL allocated and > > > cleared out pages (__GFP_ZERO) are required for page tables on any given > > > architecture. This creates a new generic GFP flag flag which can be used > > > for any page table page allocation. Does not cause any functional change. > > > > > > GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the > > > generic page tabe header just to prevent it's potential misuse as a > > > general > > > allocation flag if included in include/linux/gfp.h. > > > > I haven't reviewed the patch yet but I am wondering whether this is > > really worth it without going all the way down to unify the common code > > and remove much more code duplication. Or is this not possible for some > > reason? > > Exactly what I suggested doing in response to v1. > > Also, the approach taken here is crazy. x86 has a feature that no other > architecture has bothered to implement yet -- accounting page tables > to the process. Yet instead of spreading that goodness to all other > architectures, Anshuman has gone to more effort to avoid doing that. Also, s390 (of course, who else) has a fun custom page-table allocator for reasons that don't apply to many other archs.
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed, Jan 16, 2019 at 07:57:03AM +0100, Michal Hocko wrote: > On Wed 16-01-19 11:51:32, Anshuman Khandual wrote: > > All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL | > > __GFP_ZERO) and using it for allocating page table pages. This causes some > > code duplication which can be easily avoided. GFP_KERNEL allocated and > > cleared out pages (__GFP_ZERO) are required for page tables on any given > > architecture. This creates a new generic GFP flag flag which can be used > > for any page table page allocation. Does not cause any functional change. > > > > GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the > > generic page tabe header just to prevent it's potential misuse as a general > > allocation flag if included in include/linux/gfp.h. > > I haven't reviewed the patch yet but I am wondering whether this is > really worth it without going all the way down to unify the common code > and remove much more code duplication. Or is this not possible for some > reason? Exactly what I suggested doing in response to v1. Also, the approach taken here is crazy. x86 has a feature that no other architecture has bothered to implement yet -- accounting page tables to the process. Yet instead of spreading that goodness to all other architectures, Anshuman has gone to more effort to avoid doing that.
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
Le 16/01/2019 à 07:21, Anshuman Khandual a écrit :
All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL |
__GFP_ZERO) and using it for allocating page table pages. This causes some
code duplication which can be easily avoided. GFP_KERNEL allocated and
cleared out pages (__GFP_ZERO) are required for page tables on any given
architecture. This creates a new generic GFP flag flag which can be used
for any page table page allocation. Does not cause any functional change.
GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the
generic page tabe header just to prevent it's potential misuse as a general
allocation flag if included in include/linux/gfp.h.
Signed-off-by: Anshuman Khandual
---
Build tested on arm, arm64, powerpc, powerpc64le and x86.
Boot tested on arm64 and x86.
Changes in V2:
- Moved GFP_PGTABLE into include/asm-generic/pgtable.h
- On X86 added __GFP_ACCOUNT into GFP_PGTABLE at various places
- Replaced possible flags on riscv and nds32 with GFP_PGTABLE
Could also replace the flags in
arch/powerpc/include/asm/nohash/64/pgalloc.h with GFP_PGTABLE in
pte_alloc_one_kernel() and pte_alloc_one()
Christophe
Original V1: https://lkml.org/lkml/2019/1/12/54
arch/arm/include/asm/pgalloc.h | 8 +++-
arch/arm/mm/mmu.c| 2 +-
arch/arm64/include/asm/pgalloc.h | 9 -
arch/arm64/mm/mmu.c | 2 +-
arch/arm64/mm/pgd.c | 4 ++--
arch/nds32/include/asm/pgalloc.h | 3 +--
arch/powerpc/include/asm/book3s/64/pgalloc.h | 6 +++---
arch/powerpc/include/asm/pgalloc.h | 2 --
arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
arch/powerpc/mm/pgtable-frag.c | 4 ++--
arch/riscv/include/asm/pgalloc.h | 8 +++-
arch/sh/mm/pgtable.c | 6 ++
arch/unicore32/include/asm/pgalloc.h | 6 ++
arch/x86/kernel/espfix_64.c | 6 ++
arch/x86/mm/pgtable.c| 15 +++
include/asm-generic/pgtable.h| 2 ++
virt/kvm/arm/mmu.c | 2 +-
17 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 17ab72f..72be6f5 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -57,8 +57,6 @@ static inline void pud_populate(struct mm_struct *mm, pud_t
*pud, pmd_t *pmd)
extern pgd_t *pgd_alloc(struct mm_struct *mm);
extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
-#define PGALLOC_GFP (GFP_KERNEL | __GFP_ZERO)
-
static inline void clean_pte_table(pte_t *pte)
{
clean_dcache_area(pte + PTE_HWTABLE_PTRS, PTE_HWTABLE_SIZE);
@@ -85,7 +83,7 @@ pte_alloc_one_kernel(struct mm_struct *mm)
{
pte_t *pte;
- pte = (pte_t *)__get_free_page(PGALLOC_GFP);
+ pte = (pte_t *)__get_free_page(GFP_PGTABLE);
if (pte)
clean_pte_table(pte);
@@ -98,9 +96,9 @@ pte_alloc_one(struct mm_struct *mm)
struct page *pte;
#ifdef CONFIG_HIGHPTE
- pte = alloc_pages(PGALLOC_GFP | __GFP_HIGHMEM, 0);
+ pte = alloc_pages(GFP_PGTABLE | __GFP_HIGHMEM, 0);
#else
- pte = alloc_pages(PGALLOC_GFP, 0);
+ pte = alloc_pages(GFP_PGTABLE, 0);
#endif
if (!pte)
return NULL;
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index f5cc1cc..6d47784 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -733,7 +733,7 @@ static void __init *early_alloc(unsigned long sz)
static void *__init late_alloc(unsigned long sz)
{
- void *ptr = (void *)__get_free_pages(PGALLOC_GFP, get_order(sz));
+ void *ptr = (void *)__get_free_pages(GFP_PGTABLE, get_order(sz));
if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
BUG();
diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
index 52fa47c..d5c75bf 100644
--- a/arch/arm64/include/asm/pgalloc.h
+++ b/arch/arm64/include/asm/pgalloc.h
@@ -26,14 +26,13 @@
#define check_pgt_cache() do { } while (0)
-#define PGALLOC_GFP (GFP_KERNEL | __GFP_ZERO)
#define PGD_SIZE (PTRS_PER_PGD * sizeof(pgd_t))
#if CONFIG_PGTABLE_LEVELS > 2
static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
{
- return (pmd_t *)__get_free_page(PGALLOC_GFP);
+ return (pmd_t *)__get_free_page(GFP_PGTABLE);
}
static inline void pmd_free(struct mm_struct *mm, pmd_t *pmdp)
@@ -62,7 +61,7 @@ static inline void __pud_populate(pud_t *pudp, phys_addr_t
pmdp, pudval_t prot)
static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
{
- return (pud_t *)__get_free_page(PGALLOC_GFP);
+ return (pud_t *)__get_free_page(GFP_PGTABLE);
}
static inline void pud_free(struct mm_struct *mm, pud_t *pudp)
@@ -93,7 +92,7
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed 16-01-19 11:51:32, Anshuman Khandual wrote:
> All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL |
> __GFP_ZERO) and using it for allocating page table pages. This causes some
> code duplication which can be easily avoided. GFP_KERNEL allocated and
> cleared out pages (__GFP_ZERO) are required for page tables on any given
> architecture. This creates a new generic GFP flag flag which can be used
> for any page table page allocation. Does not cause any functional change.
>
> GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the
> generic page tabe header just to prevent it's potential misuse as a general
> allocation flag if included in include/linux/gfp.h.
I haven't reviewed the patch yet but I am wondering whether this is
really worth it without going all the way down to unify the common code
and remove much more code duplication. Or is this not possible for some
reason?
> Signed-off-by: Anshuman Khandual
> ---
> Build tested on arm, arm64, powerpc, powerpc64le and x86.
> Boot tested on arm64 and x86.
>
> Changes in V2:
>
> - Moved GFP_PGTABLE into include/asm-generic/pgtable.h
> - On X86 added __GFP_ACCOUNT into GFP_PGTABLE at various places
> - Replaced possible flags on riscv and nds32 with GFP_PGTABLE
>
> Original V1: https://lkml.org/lkml/2019/1/12/54
>
> arch/arm/include/asm/pgalloc.h | 8 +++-
> arch/arm/mm/mmu.c| 2 +-
> arch/arm64/include/asm/pgalloc.h | 9 -
> arch/arm64/mm/mmu.c | 2 +-
> arch/arm64/mm/pgd.c | 4 ++--
> arch/nds32/include/asm/pgalloc.h | 3 +--
> arch/powerpc/include/asm/book3s/64/pgalloc.h | 6 +++---
> arch/powerpc/include/asm/pgalloc.h | 2 --
> arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
> arch/powerpc/mm/pgtable-frag.c | 4 ++--
> arch/riscv/include/asm/pgalloc.h | 8 +++-
> arch/sh/mm/pgtable.c | 6 ++
> arch/unicore32/include/asm/pgalloc.h | 6 ++
> arch/x86/kernel/espfix_64.c | 6 ++
> arch/x86/mm/pgtable.c| 15 +++
> include/asm-generic/pgtable.h| 2 ++
> virt/kvm/arm/mmu.c | 2 +-
> 17 files changed, 37 insertions(+), 50 deletions(-)
>
> diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
> index 17ab72f..72be6f5 100644
> --- a/arch/arm/include/asm/pgalloc.h
> +++ b/arch/arm/include/asm/pgalloc.h
> @@ -57,8 +57,6 @@ static inline void pud_populate(struct mm_struct *mm, pud_t
> *pud, pmd_t *pmd)
> extern pgd_t *pgd_alloc(struct mm_struct *mm);
> extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
>
> -#define PGALLOC_GFP (GFP_KERNEL | __GFP_ZERO)
> -
> static inline void clean_pte_table(pte_t *pte)
> {
> clean_dcache_area(pte + PTE_HWTABLE_PTRS, PTE_HWTABLE_SIZE);
> @@ -85,7 +83,7 @@ pte_alloc_one_kernel(struct mm_struct *mm)
> {
> pte_t *pte;
>
> - pte = (pte_t *)__get_free_page(PGALLOC_GFP);
> + pte = (pte_t *)__get_free_page(GFP_PGTABLE);
> if (pte)
> clean_pte_table(pte);
>
> @@ -98,9 +96,9 @@ pte_alloc_one(struct mm_struct *mm)
> struct page *pte;
>
> #ifdef CONFIG_HIGHPTE
> - pte = alloc_pages(PGALLOC_GFP | __GFP_HIGHMEM, 0);
> + pte = alloc_pages(GFP_PGTABLE | __GFP_HIGHMEM, 0);
> #else
> - pte = alloc_pages(PGALLOC_GFP, 0);
> + pte = alloc_pages(GFP_PGTABLE, 0);
> #endif
> if (!pte)
> return NULL;
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index f5cc1cc..6d47784 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -733,7 +733,7 @@ static void __init *early_alloc(unsigned long sz)
>
> static void *__init late_alloc(unsigned long sz)
> {
> - void *ptr = (void *)__get_free_pages(PGALLOC_GFP, get_order(sz));
> + void *ptr = (void *)__get_free_pages(GFP_PGTABLE, get_order(sz));
>
> if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
> BUG();
> diff --git a/arch/arm64/include/asm/pgalloc.h
> b/arch/arm64/include/asm/pgalloc.h
> index 52fa47c..d5c75bf 100644
> --- a/arch/arm64/include/asm/pgalloc.h
> +++ b/arch/arm64/include/asm/pgalloc.h
> @@ -26,14 +26,13 @@
>
> #define check_pgt_cache()do { } while (0)
>
> -#define PGALLOC_GFP (GFP_KERNEL | __GFP_ZERO)
> #define PGD_SIZE (PTRS_PER_PGD * sizeof(pgd_t))
>
> #if CONFIG_PGTABLE_LEVELS > 2
>
> static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> - return (pmd_t *)__get_free_page(PGALLOC_GFP);
> + return (pmd_t *)__get_free_page(GFP_PGTABLE);
> }
>
> static inline void pmd_free(struct mm_struct *mm, pmd_t *pmdp)
> @@ -62,7 +61,7 @@ static inline void __pud_populate(pud_t *pudp, phys_addr_t
> pmdp, pudval_t prot)
>
> static inline pud_t *pud_alloc_one(struct mm_struct *mm,
Re: [PATCH V2] mm: Introduce GFP_PGTABLE
On Wed, Jan 16, 2019 at 11:51:32AM +0530, Anshuman Khandual wrote:
> All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL |
> __GFP_ZERO) and using it for allocating page table pages. This causes some
> code duplication which can be easily avoided. GFP_KERNEL allocated and
> cleared out pages (__GFP_ZERO) are required for page tables on any given
> architecture. This creates a new generic GFP flag flag which can be used
> for any page table page allocation. Does not cause any functional change.
>
> GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the
> generic page tabe header just to prevent it's potential misuse as a general
> allocation flag if included in include/linux/gfp.h.
>
> Signed-off-by: Anshuman Khandual
> ---
> Build tested on arm, arm64, powerpc, powerpc64le and x86.
> Boot tested on arm64 and x86.
>
> Changes in V2:
>
> - Moved GFP_PGTABLE into include/asm-generic/pgtable.h
> - On X86 added __GFP_ACCOUNT into GFP_PGTABLE at various places
> - Replaced possible flags on riscv and nds32 with GFP_PGTABLE
>
> Original V1: https://lkml.org/lkml/2019/1/12/54
>
> arch/arm/include/asm/pgalloc.h | 8 +++-
> arch/arm/mm/mmu.c| 2 +-
> arch/arm64/include/asm/pgalloc.h | 9 -
> arch/arm64/mm/mmu.c | 2 +-
> arch/arm64/mm/pgd.c | 4 ++--
> arch/nds32/include/asm/pgalloc.h | 3 +--
> arch/powerpc/include/asm/book3s/64/pgalloc.h | 6 +++---
> arch/powerpc/include/asm/pgalloc.h | 2 --
> arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
> arch/powerpc/mm/pgtable-frag.c | 4 ++--
> arch/riscv/include/asm/pgalloc.h | 8 +++-
> arch/sh/mm/pgtable.c | 6 ++
> arch/unicore32/include/asm/pgalloc.h | 6 ++
> arch/x86/kernel/espfix_64.c | 6 ++
> arch/x86/mm/pgtable.c| 15 +++
> include/asm-generic/pgtable.h| 2 ++
> virt/kvm/arm/mmu.c | 2 +-
> 17 files changed, 37 insertions(+), 50 deletions(-)
I wonder, what about the other arches? Do they use different GFP flags?
> diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
> index 17ab72f..72be6f5 100644
> --- a/arch/arm/include/asm/pgalloc.h
> +++ b/arch/arm/include/asm/pgalloc.h
> @@ -57,8 +57,6 @@ static inline void pud_populate(struct mm_struct *mm, pud_t
> *pud, pmd_t *pmd)
> extern pgd_t *pgd_alloc(struct mm_struct *mm);
> extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
>
> -#define PGALLOC_GFP (GFP_KERNEL | __GFP_ZERO)
> -
> static inline void clean_pte_table(pte_t *pte)
> {
> clean_dcache_area(pte + PTE_HWTABLE_PTRS, PTE_HWTABLE_SIZE);
> @@ -85,7 +83,7 @@ pte_alloc_one_kernel(struct mm_struct *mm)
> {
> pte_t *pte;
>
> - pte = (pte_t *)__get_free_page(PGALLOC_GFP);
> + pte = (pte_t *)__get_free_page(GFP_PGTABLE);
> if (pte)
> clean_pte_table(pte);
>
> @@ -98,9 +96,9 @@ pte_alloc_one(struct mm_struct *mm)
> struct page *pte;
>
> #ifdef CONFIG_HIGHPTE
> - pte = alloc_pages(PGALLOC_GFP | __GFP_HIGHMEM, 0);
> + pte = alloc_pages(GFP_PGTABLE | __GFP_HIGHMEM, 0);
> #else
> - pte = alloc_pages(PGALLOC_GFP, 0);
> + pte = alloc_pages(GFP_PGTABLE, 0);
> #endif
> if (!pte)
> return NULL;
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index f5cc1cc..6d47784 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -733,7 +733,7 @@ static void __init *early_alloc(unsigned long sz)
>
> static void *__init late_alloc(unsigned long sz)
> {
> - void *ptr = (void *)__get_free_pages(PGALLOC_GFP, get_order(sz));
> + void *ptr = (void *)__get_free_pages(GFP_PGTABLE, get_order(sz));
>
> if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
> BUG();
> diff --git a/arch/arm64/include/asm/pgalloc.h
> b/arch/arm64/include/asm/pgalloc.h
> index 52fa47c..d5c75bf 100644
> --- a/arch/arm64/include/asm/pgalloc.h
> +++ b/arch/arm64/include/asm/pgalloc.h
> @@ -26,14 +26,13 @@
>
> #define check_pgt_cache()do { } while (0)
>
> -#define PGALLOC_GFP (GFP_KERNEL | __GFP_ZERO)
> #define PGD_SIZE (PTRS_PER_PGD * sizeof(pgd_t))
>
> #if CONFIG_PGTABLE_LEVELS > 2
>
> static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> - return (pmd_t *)__get_free_page(PGALLOC_GFP);
> + return (pmd_t *)__get_free_page(GFP_PGTABLE);
> }
>
> static inline void pmd_free(struct mm_struct *mm, pmd_t *pmdp)
> @@ -62,7 +61,7 @@ static inline void __pud_populate(pud_t *pudp, phys_addr_t
> pmdp, pudval_t prot)
>
> static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> - return (pud_t *)__get_free_page(PGALLOC_GFP);
> + return (pud_t *)__get_free_page(GFP_PGTABLE);
> }
>
>
[PATCH V2] mm: Introduce GFP_PGTABLE
All architectures have been defining their own PGALLOC_GFP as (GFP_KERNEL |
__GFP_ZERO) and using it for allocating page table pages. This causes some
code duplication which can be easily avoided. GFP_KERNEL allocated and
cleared out pages (__GFP_ZERO) are required for page tables on any given
architecture. This creates a new generic GFP flag flag which can be used
for any page table page allocation. Does not cause any functional change.
GFP_PGTABLE is being added into include/asm-generic/pgtable.h which is the
generic page tabe header just to prevent it's potential misuse as a general
allocation flag if included in include/linux/gfp.h.
Signed-off-by: Anshuman Khandual
---
Build tested on arm, arm64, powerpc, powerpc64le and x86.
Boot tested on arm64 and x86.
Changes in V2:
- Moved GFP_PGTABLE into include/asm-generic/pgtable.h
- On X86 added __GFP_ACCOUNT into GFP_PGTABLE at various places
- Replaced possible flags on riscv and nds32 with GFP_PGTABLE
Original V1: https://lkml.org/lkml/2019/1/12/54
arch/arm/include/asm/pgalloc.h | 8 +++-
arch/arm/mm/mmu.c| 2 +-
arch/arm64/include/asm/pgalloc.h | 9 -
arch/arm64/mm/mmu.c | 2 +-
arch/arm64/mm/pgd.c | 4 ++--
arch/nds32/include/asm/pgalloc.h | 3 +--
arch/powerpc/include/asm/book3s/64/pgalloc.h | 6 +++---
arch/powerpc/include/asm/pgalloc.h | 2 --
arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
arch/powerpc/mm/pgtable-frag.c | 4 ++--
arch/riscv/include/asm/pgalloc.h | 8 +++-
arch/sh/mm/pgtable.c | 6 ++
arch/unicore32/include/asm/pgalloc.h | 6 ++
arch/x86/kernel/espfix_64.c | 6 ++
arch/x86/mm/pgtable.c| 15 +++
include/asm-generic/pgtable.h| 2 ++
virt/kvm/arm/mmu.c | 2 +-
17 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 17ab72f..72be6f5 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -57,8 +57,6 @@ static inline void pud_populate(struct mm_struct *mm, pud_t
*pud, pmd_t *pmd)
extern pgd_t *pgd_alloc(struct mm_struct *mm);
extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
-#define PGALLOC_GFP(GFP_KERNEL | __GFP_ZERO)
-
static inline void clean_pte_table(pte_t *pte)
{
clean_dcache_area(pte + PTE_HWTABLE_PTRS, PTE_HWTABLE_SIZE);
@@ -85,7 +83,7 @@ pte_alloc_one_kernel(struct mm_struct *mm)
{
pte_t *pte;
- pte = (pte_t *)__get_free_page(PGALLOC_GFP);
+ pte = (pte_t *)__get_free_page(GFP_PGTABLE);
if (pte)
clean_pte_table(pte);
@@ -98,9 +96,9 @@ pte_alloc_one(struct mm_struct *mm)
struct page *pte;
#ifdef CONFIG_HIGHPTE
- pte = alloc_pages(PGALLOC_GFP | __GFP_HIGHMEM, 0);
+ pte = alloc_pages(GFP_PGTABLE | __GFP_HIGHMEM, 0);
#else
- pte = alloc_pages(PGALLOC_GFP, 0);
+ pte = alloc_pages(GFP_PGTABLE, 0);
#endif
if (!pte)
return NULL;
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index f5cc1cc..6d47784 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -733,7 +733,7 @@ static void __init *early_alloc(unsigned long sz)
static void *__init late_alloc(unsigned long sz)
{
- void *ptr = (void *)__get_free_pages(PGALLOC_GFP, get_order(sz));
+ void *ptr = (void *)__get_free_pages(GFP_PGTABLE, get_order(sz));
if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
BUG();
diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
index 52fa47c..d5c75bf 100644
--- a/arch/arm64/include/asm/pgalloc.h
+++ b/arch/arm64/include/asm/pgalloc.h
@@ -26,14 +26,13 @@
#define check_pgt_cache() do { } while (0)
-#define PGALLOC_GFP(GFP_KERNEL | __GFP_ZERO)
#define PGD_SIZE (PTRS_PER_PGD * sizeof(pgd_t))
#if CONFIG_PGTABLE_LEVELS > 2
static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
{
- return (pmd_t *)__get_free_page(PGALLOC_GFP);
+ return (pmd_t *)__get_free_page(GFP_PGTABLE);
}
static inline void pmd_free(struct mm_struct *mm, pmd_t *pmdp)
@@ -62,7 +61,7 @@ static inline void __pud_populate(pud_t *pudp, phys_addr_t
pmdp, pudval_t prot)
static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
{
- return (pud_t *)__get_free_page(PGALLOC_GFP);
+ return (pud_t *)__get_free_page(GFP_PGTABLE);
}
static inline void pud_free(struct mm_struct *mm, pud_t *pudp)
@@ -93,7 +92,7 @@ extern void pgd_free(struct mm_struct *mm, pgd_t *pgdp);
static inline pte_t *
pte_alloc_one_kernel(struct mm_struct *mm)
{
- return (pte_t *)__get_free_page(PGALLOC_GFP);
+ return (pte_t *)__get_free_page(GFP_PGTABLE);
}
