[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0b549ff0-b0b6-4fc8-aa6f-0d76157575b3@redhat.com>
Date: Tue, 2 Jul 2024 13:02:28 +0200
From: David Hildenbrand <david@...hat.com>
To: Alistair Popple <apopple@...dia.com>
Cc: dan.j.williams@...el.com, vishal.l.verma@...el.com, dave.jiang@...el.com,
logang@...tatee.com, bhelgaas@...gle.com, jack@...e.cz, jgg@...pe.ca,
catalin.marinas@....com, will@...nel.org, mpe@...erman.id.au,
npiggin@...il.com, dave.hansen@...ux.intel.com, ira.weiny@...el.com,
willy@...radead.org, djwong@...nel.org, tytso@....edu, linmiaohe@...wei.com,
peterx@...hat.com, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org,
nvdimm@...ts.linux.dev, linux-cxl@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org, jhubbard@...dia.com,
hch@....de, david@...morbit.com
Subject: Re: [PATCH 07/13] huge_memory: Allow mappings of PUD sized pages
On 02.07.24 12:19, Alistair Popple wrote:
>
> David Hildenbrand <david@...hat.com> writes:
>
>> On 27.06.24 02:54, Alistair Popple wrote:
>>> Currently DAX folio/page reference counts are managed differently to
>>> normal pages. To allow these to be managed the same as normal pages
>>> introduce dax_insert_pfn_pud. This will map the entire PUD-sized folio
>>> and take references as it would for a normally mapped page.
>>> This is distinct from the current mechanism, vmf_insert_pfn_pud,
>>> which
>>> simply inserts a special devmap PUD entry into the page table without
>>> holding a reference to the page for the mapping.
>>
>> Do we really have to involve mapcounts/rmap for daxfs pages at this
>> point? Or is this only "to make it look more like other pages" ?
>
> The aim of the series is make FS DAX and other ZONE_DEVICE pages look
> like other pages, at least with regards to the way they are refcounted.
>
> At the moment they are not refcounted - instead their refcounts are
> basically statically initialised to one and there are all these special
> cases and functions requiring magic PTE bits (pXX_devmap) to do the
> special DAX reference counting. This then adds some cruft to manage
> pgmap references and to catch the 2->1 page refcount transition. All
> this just goes away if we manage the page references the same as other
> pages (and indeed we already manage DEVICE_PRIVATE and COHERENT pages
> the same as normal pages).
>
> So I think to make this work we at least need the mapcounts.
>
We only really need the mapcounts if we intend to do something like
folio_mapcount() == folio_ref_count() to detect unexpected folio
references, and if we have to have things like folio_mapped() working.
For now that was not required, that's why I am asking.
Background also being that in a distant future folios will be decoupled
more from other compound pages, and only folios (or "struct anon_folio"
/ "struct file_folio") would even have mapcounts.
For example, most stuff we map (and refcount!) via vm_insert_page()
really must stop involving mapcounts. These won't be "ordinary"
mapcount-tracked folios in the future, they are simply some refcounted
pages some ordinary driver allocated.
For FS-DAX, if we'll be using the same "struct file_folio" approach as
for ordinary pageache memory, then this is the right thing to do here.
>> I'm asking this because:
>>
>> (A) We don't support mixing PUD+PMD mappings yet. I have plans to change
>> that in the future, but for now you can only map using a single PUD
>> or by PTEs. I suspect that's good enoug for now for dax fs?
>
> Yep, that's all we support.
>
>> (B) As long as we have subpage mapcounts, this prevents vmemmap
>> optimizations [1]. Is that only used for device-dax for now and are
>> there no plans to make use of that for fs-dax?
>
> I don't have any plans to. This is purely focussed on refcounting pages
> "like normal" so we can get rid of all the DAX special casing.
>
>> (C) We managed without so far :)
>
> Indeed, although Christoph has asked repeatedly ([1], [2] and likely
> others) that this gets fixed and I finally got sick of it coming up
> everytime I need to touch something with ZONE_DEVICE pages :)
>
> Also it removes the need for people to understand the special DAX page
> recounting scheme and ends up removing a bunch of cruft as a bonus:
>
> 59 files changed, 485 insertions(+), 869 deletions(-)
I'm not challenging the refcounting scheme. I'm purely asking about
mapcount handling, which is something related but different.
>
> And that's before I clean up all the pgmap reference handling. It also
> removes the pXX_trans_huge and pXX_leaf distinction. So we managed, but
> things could be better IMHO.
>
Again, all nice things.
>> Having that said, with folio->_large_mapcount things like
>> folio_mapcount() are no longer terribly slow once we weould PTE-map a
>> PUD-sized folio.
>>
>> Also, all ZONE_DEVICE pages should currently be marked PG_reserved,
>> translating to "don't touch the memmap". I think we might want to
>> tackle that first.
Missed to add a pointer to [2].
>
> Ok. I'm keen to get this series finished and I don't quite get the
> connection here, what needs to change there?
include/linux/page-flags.h
"PG_reserved is set for special pages. The "struct page" of such a page
should in general not be touched (e.g. set dirty) except by its owner.
Pages marked as PG_reserved include:
...
- Device memory (e.g. PMEM, DAX, HMM)
"
I think we already entered that domain with other ZONE_DEVICE pages
being returned from vm_normal_folio(), unfortunately. But that really
must be cleaned up for these pages to not look special anymore.
Agreed that it likely is something that is not blocking this series.
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists