| Message ID | 20230612210423.18611-5-vishal.moola@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Split ptdesc from struct page | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes, riscv/for-next or riscv/master |
On Mon, Jun 12, 2023 at 02:03:53PM -0700, Vishal Moola (Oracle) wrote: > Currently, page table information is stored within struct page. As part > of simplifying struct page, create struct ptdesc for page table > information. > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com> Acked-by: Mike Rapoport (IBM) <rppt@kernel.org> > --- > include/linux/pgtable.h | 51 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > index c5a51481bbb9..330de96ebfd6 100644 > --- a/include/linux/pgtable.h > +++ b/include/linux/pgtable.h > @@ -975,6 +975,57 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, > #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ > #endif /* CONFIG_MMU */ > > + > +/** > + * struct ptdesc - Memory descriptor for page tables. > + * @__page_flags: Same as page flags. Unused for page tables. > + * @pt_list: List of used page tables. Used for s390 and x86. > + * @_pt_pad_1: Padding that aliases with page's compound head. > + * @pmd_huge_pte: Protected by ptdesc->ptl, used for THPs. > + * @_pt_s390_gaddr: Aliases with page's mapping. Used for s390 gmap only. > + * @pt_mm: Used for x86 pgds. > + * @pt_frag_refcount: For fragmented page table tracking. Powerpc and s390 only. > + * @ptl: Lock for the page table. Do you mind aligning the descriptions by @pt_frag_refcount? I think it'll be more readable. > + * > + * This struct overlays struct page for now. Do not modify without a good > + * understanding of the issues. > + */ > +struct ptdesc { > + unsigned long __page_flags; > + > + union { > + struct list_head pt_list; > + struct { > + unsigned long _pt_pad_1; > + pgtable_t pmd_huge_pte; > + }; > + }; > + unsigned long _pt_s390_gaddr; > + > + union { > + struct mm_struct *pt_mm; > + atomic_t pt_frag_refcount; > + }; > + > +#if ALLOC_SPLIT_PTLOCKS > + spinlock_t *ptl; > +#else > + spinlock_t ptl; > +#endif > +}; > + > +#define TABLE_MATCH(pg, pt) \ > + static_assert(offsetof(struct page, pg) == offsetof(struct ptdesc, pt)) > +TABLE_MATCH(flags, __page_flags); > +TABLE_MATCH(compound_head, pt_list); > +TABLE_MATCH(compound_head, _pt_pad_1); > +TABLE_MATCH(pmd_huge_pte, pmd_huge_pte); > +TABLE_MATCH(mapping, _pt_s390_gaddr); > +TABLE_MATCH(pt_mm, pt_mm); > +TABLE_MATCH(ptl, ptl); > +#undef TABLE_MATCH > +static_assert(sizeof(struct ptdesc) <= sizeof(struct page)); > + > /* > * No-op macros that just return the current protection value. Defined here > * because these macros can be used even if CONFIG_MMU is not defined. > -- > 2.40.1 > >
On Mon, 12 Jun 2023, Vishal Moola (Oracle) wrote: > Currently, page table information is stored within struct page. As part > of simplifying struct page, create struct ptdesc for page table > information. > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com> Vishal, as I think you have already guessed, your ptdesc series and my pte_free_defer() "mm: free retracted page table by RCU" series are on a collision course. Probably just trivial collisions in most architectures, which either of us can easily adjust to the other; powerpc likely to be more awkward, but fairly easily resolved; s390 quite a problem. I've so far been unable to post a v2 of my series (and powerpc and s390 were stupidly wrong in the v1), because a good s390 patch is not yet decided - Gerald Schaefer and I are currently working on that, on the s390 list (I took off most Ccs until we are settled and I can post v2). As you have no doubt found yourself, s390 has sophisticated handling of free half-pages already, and I need to add rcu_head usage in there too: it's tricky to squeeze it all in, and ptdesc does not appear to help us in any way (though mostly it's just changing some field names, okay). If ptdesc were actually allowing a flexible structure which architectures could add into, that would (in some future) be nice; but of course at present it's still fitting it all into one struct page, and mandating new restrictions which just make an architecture's job harder. Some notes on problematic fields below FYI. > --- > include/linux/pgtable.h | 51 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > index c5a51481bbb9..330de96ebfd6 100644 > --- a/include/linux/pgtable.h > +++ b/include/linux/pgtable.h > @@ -975,6 +975,57 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, > #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ > #endif /* CONFIG_MMU */ > > + > +/** > + * struct ptdesc - Memory descriptor for page tables. > + * @__page_flags: Same as page flags. Unused for page tables. > + * @pt_list: List of used page tables. Used for s390 and x86. > + * @_pt_pad_1: Padding that aliases with page's compound head. > + * @pmd_huge_pte: Protected by ptdesc->ptl, used for THPs. > + * @_pt_s390_gaddr: Aliases with page's mapping. Used for s390 gmap only. > + * @pt_mm: Used for x86 pgds. > + * @pt_frag_refcount: For fragmented page table tracking. Powerpc and s390 only. > + * @ptl: Lock for the page table. > + * > + * This struct overlays struct page for now. Do not modify without a good > + * understanding of the issues. > + */ > +struct ptdesc { > + unsigned long __page_flags; > + > + union { > + struct list_head pt_list; I shall be needing struct rcu_head rcu_head (or pt_rcu_head or whatever, if you prefer) in this union too. Sharing the lru or pt_list with rcu_head is what's difficult to get right and efficient on s390 - and if ptdesc gave us an independent rcu_head for each page table, that would be a blessing! but sadly not, it still has to squeeze into a struct page. > + struct { > + unsigned long _pt_pad_1; > + pgtable_t pmd_huge_pte; > + }; > + }; > + unsigned long _pt_s390_gaddr; > + > + union { > + struct mm_struct *pt_mm; > + atomic_t pt_frag_refcount; Whether s390 will want pt_mm is not yet decided: I want to use it, Gerald prefers to go without it; but if we do end up using it, then pt_frag_refcount is a luxury we would have to give up. s390 does very well already with its _refcount tricks, and I'd expect powerpc's simpler but more wasteful implementation to work as well with _refcount too - I know that a few years back, powerpc did misuse _refcount (it did not allow for speculative accesses, thought it had sole ownership of that field); but s390 copes well with that, and I expect powerpc can do so too, without the luxury of pt_frag_refcount. But I've no desire to undo powerpc's use of pt_frag_refcount: just warning that we may want to undo any use of it in s390. I thought I had more issues to mention, probably Gerald will remind me of a whole new unexplored dimension! gmap perhaps. Hugh > + }; > + > +#if ALLOC_SPLIT_PTLOCKS > + spinlock_t *ptl; > +#else > + spinlock_t ptl; > +#endif > +}; > + > +#define TABLE_MATCH(pg, pt) \ > + static_assert(offsetof(struct page, pg) == offsetof(struct ptdesc, pt)) > +TABLE_MATCH(flags, __page_flags); > +TABLE_MATCH(compound_head, pt_list); > +TABLE_MATCH(compound_head, _pt_pad_1); > +TABLE_MATCH(pmd_huge_pte, pmd_huge_pte); > +TABLE_MATCH(mapping, _pt_s390_gaddr); > +TABLE_MATCH(pt_mm, pt_mm); > +TABLE_MATCH(ptl, ptl); > +#undef TABLE_MATCH > +static_assert(sizeof(struct ptdesc) <= sizeof(struct page)); > + > /* > * No-op macros that just return the current protection value. Defined here > * because these macros can be used even if CONFIG_MMU is not defined. > -- > 2.40.1
On Mon, Jun 12, 2023 at 02:03:53PM -0700, Vishal Moola (Oracle) wrote: > Currently, page table information is stored within struct page. As part > of simplifying struct page, create struct ptdesc for page table > information. > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com> > --- > include/linux/pgtable.h | 51 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > index c5a51481bbb9..330de96ebfd6 100644 > --- a/include/linux/pgtable.h > +++ b/include/linux/pgtable.h > @@ -975,6 +975,57 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, > #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ > #endif /* CONFIG_MMU */ > > + > +/** > + * struct ptdesc - Memory descriptor for page tables. > + * @__page_flags: Same as page flags. Unused for page tables. > + * @pt_list: List of used page tables. Used for s390 and x86. > + * @_pt_pad_1: Padding that aliases with page's compound head. > + * @pmd_huge_pte: Protected by ptdesc->ptl, used for THPs. > + * @_pt_s390_gaddr: Aliases with page's mapping. Used for s390 gmap only. > + * @pt_mm: Used for x86 pgds. > + * @pt_frag_refcount: For fragmented page table tracking. Powerpc and s390 only. > + * @ptl: Lock for the page table. > + * > + * This struct overlays struct page for now. Do not modify without a good > + * understanding of the issues. > + */ > +struct ptdesc { > + unsigned long __page_flags; > + > + union { > + struct list_head pt_list; > + struct { > + unsigned long _pt_pad_1; > + pgtable_t pmd_huge_pte; > + }; > + }; > + unsigned long _pt_s390_gaddr; > + > + union { > + struct mm_struct *pt_mm; > + atomic_t pt_frag_refcount; > + }; > + > +#if ALLOC_SPLIT_PTLOCKS > + spinlock_t *ptl; > +#else > + spinlock_t ptl; > +#endif > +}; I think you should include the memcg here too? It needs to be valid for a ptdesc, even if we don't currently deref it through the ptdesc type. Also, do you see a way to someday put a 'struct rcu_head' into here? Thanks, Jason
On Thu, Jun 15, 2023 at 12:57:19AM -0700, Hugh Dickins wrote: > Probably just trivial collisions in most architectures, which either > of us can easily adjust to the other; powerpc likely to be more awkward, > but fairly easily resolved; s390 quite a problem. > > I've so far been unable to post a v2 of my series (and powerpc and s390 > were stupidly wrong in the v1), because a good s390 patch is not yet > decided - Gerald Schaefer and I are currently working on that, on the > s390 list (I took off most Ccs until we are settled and I can post v2). > > As you have no doubt found yourself, s390 has sophisticated handling of > free half-pages already, and I need to add rcu_head usage in there too: > it's tricky to squeeze it all in, and ptdesc does not appear to help us > in any way (though mostly it's just changing some field names, okay). > > If ptdesc were actually allowing a flexible structure which architectures > could add into, that would (in some future) be nice; but of course at > present it's still fitting it all into one struct page, and mandating > new restrictions which just make an architecture's job harder. The intent is to get ptdescs to be dynamically allocated at some point in the ~2-3 years out future when we have finished the folio project ... which is not a terribly helpful thing for me to say. I have three suggestions, probably all dreadful: 1. s390 could change its behaviour to always allocate page tables in pairs. That is, it fills in two pmd_t entries any time it takes a fault in either of them. 2. We could allocate two or four pages at a time for s390 to allocate 2kB pages from. That gives us a lot more space to store RCU heads. 3. We could use s390 as a guinea-pig for dynamic ptdesc allocation. Every time we allocate a struct page, we have a slab cache for an s390-special definition of struct ptdesc, we allocate a ptdesc and store a pointer to that in compound_head. We could sweeten #3 by doing that not just for s390 but also for every configuration which has ALLOC_SPLIT_PTLOCKS today. That would get rid of the ambiguity between "is ptl a pointer or a lock". > But I've no desire to undo powerpc's use of pt_frag_refcount: > just warning that we may want to undo any use of it in s390. I would dearly love ppc & s390 to use the _same_ scheme to solve the same problem.
On Thu, Jun 15, 2023 at 12:57 AM Hugh Dickins <hughd@google.com> wrote: > > On Mon, 12 Jun 2023, Vishal Moola (Oracle) wrote: > > > Currently, page table information is stored within struct page. As part > > of simplifying struct page, create struct ptdesc for page table > > information. > > > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com> > > Vishal, as I think you have already guessed, your ptdesc series and > my pte_free_defer() "mm: free retracted page table by RCU" series are > on a collision course. > > Probably just trivial collisions in most architectures, which either > of us can easily adjust to the other; powerpc likely to be more awkward, > but fairly easily resolved; s390 quite a problem. > > I've so far been unable to post a v2 of my series (and powerpc and s390 > were stupidly wrong in the v1), because a good s390 patch is not yet > decided - Gerald Schaefer and I are currently working on that, on the > s390 list (I took off most Ccs until we are settled and I can post v2). > > As you have no doubt found yourself, s390 has sophisticated handling of > free half-pages already, and I need to add rcu_head usage in there too: > it's tricky to squeeze it all in, and ptdesc does not appear to help us > in any way (though mostly it's just changing some field names, okay). > > If ptdesc were actually allowing a flexible structure which architectures > could add into, that would (in some future) be nice; but of course at > present it's still fitting it all into one struct page, and mandating > new restrictions which just make an architecture's job harder. A goal of ptdescs is to make architecture's jobs simpler and standardized. Unfortunately, ptdescs are nowhere near isolated from struct page yet. This version of struct ptdesc contains the exact number of fields architectures need right now, just reorganized to be located next to each other. It *probably* shouldn't make an architectures job harder, aside from discouraging their use of yet even more members of struct page. > Some notes on problematic fields below FYI. > > > --- > > include/linux/pgtable.h | 51 +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > index c5a51481bbb9..330de96ebfd6 100644 > > --- a/include/linux/pgtable.h > > +++ b/include/linux/pgtable.h > > @@ -975,6 +975,57 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, > > #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ > > #endif /* CONFIG_MMU */ > > > > + > > +/** > > + * struct ptdesc - Memory descriptor for page tables. > > + * @__page_flags: Same as page flags. Unused for page tables. > > + * @pt_list: List of used page tables. Used for s390 and x86. > > + * @_pt_pad_1: Padding that aliases with page's compound head. > > + * @pmd_huge_pte: Protected by ptdesc->ptl, used for THPs. > > + * @_pt_s390_gaddr: Aliases with page's mapping. Used for s390 gmap only. > > + * @pt_mm: Used for x86 pgds. > > + * @pt_frag_refcount: For fragmented page table tracking. Powerpc and s390 only. > > + * @ptl: Lock for the page table. > > + * > > + * This struct overlays struct page for now. Do not modify without a good > > + * understanding of the issues. > > + */ > > +struct ptdesc { > > + unsigned long __page_flags; > > + > > + union { > > + struct list_head pt_list; > > I shall be needing struct rcu_head rcu_head (or pt_rcu_head or whatever, > if you prefer) in this union too. Sharing the lru or pt_list with rcu_head > is what's difficult to get right and efficient on s390 - and if ptdesc gave > us an independent rcu_head for each page table, that would be a blessing! > but sadly not, it still has to squeeze into a struct page. I can add a pt_rcu_head along with a comment to deter aliasing issues :) Independent rcu_heads aren't coming any time soon though :( > > + struct { > > + unsigned long _pt_pad_1; > > + pgtable_t pmd_huge_pte; > > + }; > > + }; > > + unsigned long _pt_s390_gaddr; > > + > > + union { > > + struct mm_struct *pt_mm; > > + atomic_t pt_frag_refcount; > > Whether s390 will want pt_mm is not yet decided: I want to use it, > Gerald prefers to go without it; but if we do end up using it, > then pt_frag_refcount is a luxury we would have to give up. I don't like the use of pt_mm for s390 either. s390 uses space equivalent to all five words allocated in the page table struct (albeit in various places of struct page). Using extra space (especially allocated for unrelated reasons) just because it exists makes things more complicated and confusing, and s390 is already confusing enough as a result of that. If having access to pt_mm is necessary I can drop the pt_frag_refcount patch, but I'd rather avoid it. > s390 does very well already with its _refcount tricks, and I'd expect > powerpc's simpler but more wasteful implementation to work as well > with _refcount too - I know that a few years back, powerpc did misuse > _refcount (it did not allow for speculative accesses, thought it had > sole ownership of that field); but s390 copes well with that, and I > expect powerpc can do so too, without the luxury of pt_frag_refcount. > > But I've no desire to undo powerpc's use of pt_frag_refcount: > just warning that we may want to undo any use of it in s390. > > I thought I had more issues to mention, probably Gerald will > remind me of a whole new unexplored dimension! gmap perhaps. > > Hugh > > > + }; > > + > > +#if ALLOC_SPLIT_PTLOCKS > > + spinlock_t *ptl; > > +#else > > + spinlock_t ptl; > > +#endif > > +}; > > + > > +#define TABLE_MATCH(pg, pt) \ > > + static_assert(offsetof(struct page, pg) == offsetof(struct ptdesc, pt)) > > +TABLE_MATCH(flags, __page_flags); > > +TABLE_MATCH(compound_head, pt_list); > > +TABLE_MATCH(compound_head, _pt_pad_1); > > +TABLE_MATCH(pmd_huge_pte, pmd_huge_pte); > > +TABLE_MATCH(mapping, _pt_s390_gaddr); > > +TABLE_MATCH(pt_mm, pt_mm); > > +TABLE_MATCH(ptl, ptl); > > +#undef TABLE_MATCH > > +static_assert(sizeof(struct ptdesc) <= sizeof(struct page)); > > + > > /* > > * No-op macros that just return the current protection value. Defined here > > * because these macros can be used even if CONFIG_MMU is not defined. > > -- > > 2.40.1
On Fri, Jun 16, 2023 at 5:38 AM Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Mon, Jun 12, 2023 at 02:03:53PM -0700, Vishal Moola (Oracle) wrote: > > Currently, page table information is stored within struct page. As part > > of simplifying struct page, create struct ptdesc for page table > > information. > > > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com> > > --- > > include/linux/pgtable.h | 51 +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > index c5a51481bbb9..330de96ebfd6 100644 > > --- a/include/linux/pgtable.h > > +++ b/include/linux/pgtable.h > > @@ -975,6 +975,57 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, > > #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ > > #endif /* CONFIG_MMU */ > > > > + > > +/** > > + * struct ptdesc - Memory descriptor for page tables. > > + * @__page_flags: Same as page flags. Unused for page tables. > > + * @pt_list: List of used page tables. Used for s390 and x86. > > + * @_pt_pad_1: Padding that aliases with page's compound head. > > + * @pmd_huge_pte: Protected by ptdesc->ptl, used for THPs. > > + * @_pt_s390_gaddr: Aliases with page's mapping. Used for s390 gmap only. > > + * @pt_mm: Used for x86 pgds. > > + * @pt_frag_refcount: For fragmented page table tracking. Powerpc and s390 only. > > + * @ptl: Lock for the page table. > > + * > > + * This struct overlays struct page for now. Do not modify without a good > > + * understanding of the issues. > > + */ > > +struct ptdesc { > > + unsigned long __page_flags; > > + > > + union { > > + struct list_head pt_list; > > + struct { > > + unsigned long _pt_pad_1; > > + pgtable_t pmd_huge_pte; > > + }; > > + }; > > + unsigned long _pt_s390_gaddr; > > + > > + union { > > + struct mm_struct *pt_mm; > > + atomic_t pt_frag_refcount; > > + }; > > + > > +#if ALLOC_SPLIT_PTLOCKS > > + spinlock_t *ptl; > > +#else > > + spinlock_t ptl; > > +#endif > > +}; > > I think you should include the memcg here too? It needs to be valid > for a ptdesc, even if we don't currently deref it through the ptdesc > type. Yes, thanks for catching that! I'll add it to v5. > Also, do you see a way to someday put a 'struct rcu_head' into here? Eventually, when they're being dynamically allocated independent of struct page. Although at that point I'm not sure if we'll need one. > Thanks, > Jason
On Tue, Jun 20, 2023 at 01:01:39PM -0700, Vishal Moola wrote: > On Fri, Jun 16, 2023 at 5:38 AM Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > On Mon, Jun 12, 2023 at 02:03:53PM -0700, Vishal Moola (Oracle) wrote: > > > Currently, page table information is stored within struct page. As part > > > of simplifying struct page, create struct ptdesc for page table > > > information. > > > > > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com> > > > --- > > > include/linux/pgtable.h | 51 +++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 51 insertions(+) > > > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > > index c5a51481bbb9..330de96ebfd6 100644 > > > --- a/include/linux/pgtable.h > > > +++ b/include/linux/pgtable.h > > > @@ -975,6 +975,57 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, > > > #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ > > > #endif /* CONFIG_MMU */ > > > > > > + > > > +/** > > > + * struct ptdesc - Memory descriptor for page tables. > > > + * @__page_flags: Same as page flags. Unused for page tables. > > > + * @pt_list: List of used page tables. Used for s390 and x86. > > > + * @_pt_pad_1: Padding that aliases with page's compound head. > > > + * @pmd_huge_pte: Protected by ptdesc->ptl, used for THPs. > > > + * @_pt_s390_gaddr: Aliases with page's mapping. Used for s390 gmap only. > > > + * @pt_mm: Used for x86 pgds. > > > + * @pt_frag_refcount: For fragmented page table tracking. Powerpc and s390 only. > > > + * @ptl: Lock for the page table. > > > + * > > > + * This struct overlays struct page for now. Do not modify without a good > > > + * understanding of the issues. > > > + */ > > > +struct ptdesc { > > > + unsigned long __page_flags; > > > + > > > + union { > > > + struct list_head pt_list; > > > + struct { > > > + unsigned long _pt_pad_1; > > > + pgtable_t pmd_huge_pte; > > > + }; > > > + }; > > > + unsigned long _pt_s390_gaddr; > > > + > > > + union { > > > + struct mm_struct *pt_mm; > > > + atomic_t pt_frag_refcount; > > > + }; > > > + > > > +#if ALLOC_SPLIT_PTLOCKS > > > + spinlock_t *ptl; > > > +#else > > > + spinlock_t ptl; > > > +#endif > > > +}; > > > > I think you should include the memcg here too? It needs to be valid > > for a ptdesc, even if we don't currently deref it through the ptdesc > > type. > > Yes, thanks for catching that! I'll add it to v5. > > > Also, do you see a way to someday put a 'struct rcu_head' into here? > > Eventually, when they're being dynamically allocated independent of > struct page. Although at that point I'm not sure if we'll need one. Sooner than dynamic struct page? Probably it can overlap pt_list in alot of arches? Jason
On Tue, Jun 20, 2023 at 4:05 PM Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Tue, Jun 20, 2023 at 01:01:39PM -0700, Vishal Moola wrote: > > On Fri, Jun 16, 2023 at 5:38 AM Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > > > On Mon, Jun 12, 2023 at 02:03:53PM -0700, Vishal Moola (Oracle) wrote: > > > > Currently, page table information is stored within struct page. As part > > > > of simplifying struct page, create struct ptdesc for page table > > > > information. > > > > > > > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com> > > > > --- > > > > include/linux/pgtable.h | 51 +++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 51 insertions(+) > > > > > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > > > index c5a51481bbb9..330de96ebfd6 100644 > > > > --- a/include/linux/pgtable.h > > > > +++ b/include/linux/pgtable.h > > > > @@ -975,6 +975,57 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, > > > > #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ > > > > #endif /* CONFIG_MMU */ > > > > > > > > + > > > > +/** > > > > + * struct ptdesc - Memory descriptor for page tables. > > > > + * @__page_flags: Same as page flags. Unused for page tables. > > > > + * @pt_list: List of used page tables. Used for s390 and x86. > > > > + * @_pt_pad_1: Padding that aliases with page's compound head. > > > > + * @pmd_huge_pte: Protected by ptdesc->ptl, used for THPs. > > > > + * @_pt_s390_gaddr: Aliases with page's mapping. Used for s390 gmap only. > > > > + * @pt_mm: Used for x86 pgds. > > > > + * @pt_frag_refcount: For fragmented page table tracking. Powerpc and s390 only. > > > > + * @ptl: Lock for the page table. > > > > + * > > > > + * This struct overlays struct page for now. Do not modify without a good > > > > + * understanding of the issues. > > > > + */ > > > > +struct ptdesc { > > > > + unsigned long __page_flags; > > > > + > > > > + union { > > > > + struct list_head pt_list; > > > > + struct { > > > > + unsigned long _pt_pad_1; > > > > + pgtable_t pmd_huge_pte; > > > > + }; > > > > + }; > > > > + unsigned long _pt_s390_gaddr; > > > > + > > > > + union { > > > > + struct mm_struct *pt_mm; > > > > + atomic_t pt_frag_refcount; > > > > + }; > > > > + > > > > +#if ALLOC_SPLIT_PTLOCKS > > > > + spinlock_t *ptl; > > > > +#else > > > > + spinlock_t ptl; > > > > +#endif > > > > +}; > > > > > > I think you should include the memcg here too? It needs to be valid > > > for a ptdesc, even if we don't currently deref it through the ptdesc > > > type. > > > > Yes, thanks for catching that! I'll add it to v5. > > > > > Also, do you see a way to someday put a 'struct rcu_head' into here? > > > > Eventually, when they're being dynamically allocated independent of > > struct page. Although at that point I'm not sure if we'll need one. > > Sooner than dynamic struct page? > > Probably it can overlap pt_list in alot of arches? Ah yes, there will be one if v5 overlapping with pt_list (it already does in struct page anyways).
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index c5a51481bbb9..330de96ebfd6 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -975,6 +975,57 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, #endif /* __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION */ #endif /* CONFIG_MMU */ + +/** + * struct ptdesc - Memory descriptor for page tables. + * @__page_flags: Same as page flags. Unused for page tables. + * @pt_list: List of used page tables. Used for s390 and x86. + * @_pt_pad_1: Padding that aliases with page's compound head. + * @pmd_huge_pte: Protected by ptdesc->ptl, used for THPs. + * @_pt_s390_gaddr: Aliases with page's mapping. Used for s390 gmap only. + * @pt_mm: Used for x86 pgds. + * @pt_frag_refcount: For fragmented page table tracking. Powerpc and s390 only. + * @ptl: Lock for the page table. + * + * This struct overlays struct page for now. Do not modify without a good + * understanding of the issues. + */ +struct ptdesc { + unsigned long __page_flags; + + union { + struct list_head pt_list; + struct { + unsigned long _pt_pad_1; + pgtable_t pmd_huge_pte; + }; + }; + unsigned long _pt_s390_gaddr; + + union { + struct mm_struct *pt_mm; + atomic_t pt_frag_refcount; + }; + +#if ALLOC_SPLIT_PTLOCKS + spinlock_t *ptl; +#else + spinlock_t ptl; +#endif +}; + +#define TABLE_MATCH(pg, pt) \ + static_assert(offsetof(struct page, pg) == offsetof(struct ptdesc, pt)) +TABLE_MATCH(flags, __page_flags); +TABLE_MATCH(compound_head, pt_list); +TABLE_MATCH(compound_head, _pt_pad_1); +TABLE_MATCH(pmd_huge_pte, pmd_huge_pte); +TABLE_MATCH(mapping, _pt_s390_gaddr); +TABLE_MATCH(pt_mm, pt_mm); +TABLE_MATCH(ptl, ptl); +#undef TABLE_MATCH +static_assert(sizeof(struct ptdesc) <= sizeof(struct page)); + /* * No-op macros that just return the current protection value. Defined here * because these macros can be used even if CONFIG_MMU is not defined.
Currently, page table information is stored within struct page. As part of simplifying struct page, create struct ptdesc for page table information. Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com> --- include/linux/pgtable.h | 51 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)