[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 01/33] s390: Use _pt_s390_gaddr for gmap address tracking



On 18.04.23 23:33, Vishal Moola wrote:
On Tue, Apr 18, 2023 at 8:45 AM David Hildenbrand <david@xxxxxxxxxx> 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@xxxxxxxxx>
---

[...]

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.


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.

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.

--
Thanks,

David / dhildenb




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.