Linux-csky Archive mirror
 help / color / mirror / Atom feed
From: Vishal Moola <vishal.moola@gmail.com>
To: David Hildenbrand <david@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Matthew Wilcox <willy@infradead.org>,
	linux-mm@kvack.org, linux-arch@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-csky@vger.kernel.org,
	linux-hexagon@vger.kernel.org, loongarch@lists.linux.dev,
	linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org,
	linux-openrisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org,
	linux-sh@vger.kernel.org, sparclinux@vger.kernel.org,
	linux-um@lists.infradead.org, xen-devel@lists.xenproject.org,
	kvm@vger.kernel.org
Subject: Re: [PATCH 01/33] s390: Use _pt_s390_gaddr for gmap address tracking
Date: Thu, 20 Apr 2023 16:32:50 -0700	[thread overview]
Message-ID: <CAOzc2pwDtn836Tf0Egh+Z258hxSTVtvwuyU2qiJa1iLa6vZFjQ@mail.gmail.com> (raw)
In-Reply-To: <e0c0ad67-f23f-ff35-80bf-841dcfd43d99@redhat.com>

On Wed, Apr 19, 2023 at 12:54 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 18.04.23 23:33, Vishal Moola wrote:
> > On Tue, Apr 18, 2023 at 8:45 AM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 17.04.23 22:50, Vishal Moola (Oracle) wrote:
> >>> s390 uses page->index to keep track of page tables for the guest address
> >>> space. In an attempt to consolidate the usage of page fields in s390,
> >>> replace _pt_pad_2 with _pt_s390_gaddr to replace page->index in gmap.
> >>>
> >>> This will help with the splitting of struct ptdesc from struct page, as
> >>> well as allow s390 to use _pt_frag_refcount for fragmented page table
> >>> tracking.
> >>>
> >>> Since page->_pt_s390_gaddr aliases with mapping, ensure its set to NULL
> >>> before freeing the pages as well.
> >>>
> >>> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> >>> ---
> >>
> >> [...]
> >>
> >>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> >>> index 3fc9e680f174..2616d64c0e8c 100644
> >>> --- a/include/linux/mm_types.h
> >>> +++ b/include/linux/mm_types.h
> >>> @@ -144,7 +144,7 @@ struct page {
> >>>                struct {        /* Page table pages */
> >>>                        unsigned long _pt_pad_1;        /* compound_head */
> >>>                        pgtable_t pmd_huge_pte; /* protected by page->ptl */
> >>> -                     unsigned long _pt_pad_2;        /* mapping */
> >>> +                     unsigned long _pt_s390_gaddr;   /* mapping */
> >>>                        union {
> >>>                                struct mm_struct *pt_mm; /* x86 pgds only */
> >>>                                atomic_t pt_frag_refcount; /* powerpc */
> >>
> >> The confusing part is, that these gmap page tables are not ordinary
> >> process page tables that we would ordinarily place into this section
> >> here. That's why they are also not allocated/freed using the typical
> >> page table constructor/destructor ...
> >
> > I initially thought the same, so I was quite confused when I saw
> > __gmap_segment_gaddr was using pmd_pgtable_page().
> >
> > Although they are not ordinary process page tables, since we
> > eventually want to move them out of struct page, I think shifting them
> > to be in ptdescs, being a memory descriptor for page tables, makes
> > the most sense.
>
> Seeing utilities like tlb_remove_page_ptdesc() that don't really apply
> to such page tables, I wonder if we should much rather treat such
> shadow/auxiliary/... page tables (just like other architectures like
> x86, arm, ... employ as well) as a distinct type.
>
> And have ptdesc be the common type for all process page tables.

Although I do like the idea of having a distinct type for them, I'm not sure
I see the merits of having another type specifically for those types of
page tables.

As it currently is, tlb_remove_table() is only distinct from tlb_remove_page()
when an architecture defines its own removal function. I'm not too familiar
with most of their differences, but we can probably continue to let them do
that. As of now, I'm not too sure what a distinct type would look like that
could meet all their needs holistically.

> >
> > Another option is to leave pmd_pgtable_page() as is just for this case.
> > Or we can revert commit 7e25de77bc5ea which uses the function here
> > then figure out where these gmap pages table pages will go later.
>
> I'm always confused when reading gmap code, so let me have another look :)
>
> The confusing part is that s390x shares the lowest level page tables
> (PTE tables) between the process and gmap ("guest mapping", similar to
> EPT on x86-64). It maps these process PTE tables (covering 1 MiB) into
> gmap-specific PMD tables.

Especially in cases like this. If the architecture wants to share page tables
then everything being in form ptdesc would make that easiest, and
continue to let them define their own niche functions for their needs.

> pmd_pgtable_page() should indeed always give us a gmap-specific
> PMD-table. In fact, something allocated via gmap_alloc_table().
>
> Decoupling both concepts sounds like a good idea.

Yeah, I'm not a fan of how this gmap caller is the only external caller
using this to get a page for their own purposes. I'll update that in v2.

  reply	other threads:[~2023-04-20 23:33 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-17 20:50 [PATCH 00/33] Split ptdesc from struct page Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 01/33] s390: Use _pt_s390_gaddr for gmap address tracking Vishal Moola (Oracle)
2023-04-18 15:45   ` David Hildenbrand
2023-04-18 21:33     ` Vishal Moola
2023-04-19  7:54       ` David Hildenbrand
2023-04-20 23:32         ` Vishal Moola [this message]
2023-04-17 20:50 ` [PATCH 02/33] s390: Use pt_frag_refcount for pagetables Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 03/33] pgtable: Create struct ptdesc Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 04/33] mm: add utility functions for ptdesc Vishal Moola (Oracle)
2023-04-18  1:22   ` kernel test robot
2023-04-19 13:33   ` [PATCH 4/33] " Vernon Yang
2023-04-19 20:29     ` Vishal Moola
2023-04-17 20:50 ` [PATCH 05/33] mm: Convert pmd_pgtable_page() to pmd_ptdesc() Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 06/33] mm: Convert ptlock_alloc() to use ptdescs Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 07/33] mm: Convert ptlock_ptr() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 08/33] mm: Convert pmd_ptlock_init() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 09/33] mm: Convert ptlock_init() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 10/33] mm: Convert pmd_ptlock_free() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 11/33] mm: Convert ptlock_free() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 12/33] mm: Create ptdesc equivalents for pgtable_{pte,pmd}_page_{ctor,dtor} Vishal Moola (Oracle)
2023-04-18  2:13   ` kernel test robot
2023-04-17 20:50 ` [PATCH 13/33] powerpc: Convert various functions to use ptdescs Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 14/33] x86: " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 15/33] s390: Convert various gmap " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 16/33] s390: Convert various pgalloc " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 17/33] mm: Remove page table members from struct page Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 18/33] pgalloc: Convert various functions to use ptdescs Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 19/33] arm: " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 20/33] arm64: " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 21/33] csky: Convert __pte_free_tlb() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 22/33] hexagon: " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 23/33] loongarch: Convert various functions " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 24/33] m68k: " Vishal Moola (Oracle)
2023-04-17 22:18   ` kernel test robot
2023-04-17 20:50 ` [PATCH 25/33] mips: " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 26/33] nios2: Convert __pte_free_tlb() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 27/33] openrisc: " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 28/33] riscv: Convert alloc_{pmd, pte}_late() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 29/33] sh: Convert pte_free_tlb() " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 30/33] sparc64: Convert various functions " Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 31/33] sparc: Convert pgtable_pte_page_{ctor, dtor}() to ptdesc equivalents Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 32/33] um: Convert {pmd, pte}_free_tlb() to use ptdescs Vishal Moola (Oracle)
2023-04-17 20:50 ` [PATCH 33/33] mm: Remove pgtable_{pmd, pte}_page_{ctor, dtor}() wrappers Vishal Moola (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOzc2pwDtn836Tf0Egh+Z258hxSTVtvwuyU2qiJa1iLa6vZFjQ@mail.gmail.com \
    --to=vishal.moola@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-csky@vger.kernel.org \
    --cc=linux-hexagon@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-openrisc@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=loongarch@lists.linux.dev \
    --cc=sparclinux@vger.kernel.org \
    --cc=willy@infradead.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).