Re: [PATCH 4/5] s390: Remove custom definition of mk_pte()

[Alexander Gordeev <agordeev@linux.ibm.com>]

On Wed, Aug 14, 2024 at 04:44:24PM +0100, Matthew Wilcox (Oracle) wrote:

Hi Matthew,

> I believe the test for PageDirty() is no longer needed.  The
> commit adding it was abf09bed3cce with the rationale that this
> avoided faults for tmpfs and shmem pages.  shmem does not mark
> newly allocated folios as dirty since 2016 (commit 75edd345e8ed)
> so this test has been ineffective since then.

The PageDirty() test you suggest to remove is still entered.
I initially thought that test could also be useful for other
architectures as an optimization, but at least one path we
take for shmem mapping is raising eyebrow, because it is a
read accesss:

handle_pte_fault() -> do_pte_missing() -> do_fault() ->
do_read_fault() -> finish_fault() -> set_pte_range() -> mk_pte()

A read fault causing the PTE dirtifying is something strange
and your patch alone could be a nice cleanup.

As other architectures do not do such a trick suggests that
mk_pte() + pte_mkdirty() is called from the same handler
or pte_mkdirty() is expected to be called from a follow-up
write handler.

I could not identify locations where that would not be the case,
but may be you know?

...
> -static inline pte_t mk_pte(struct page *page, pgprot_t pgprot)
> -{
> -	unsigned long physpage = page_to_phys(page);
> -	pte_t __pte = mk_pte_phys(physpage, pgprot);
> -
> -	if (pte_write(__pte) && PageDirty(page))
> -		__pte = pte_mkdirty(__pte);
> -	return __pte;
> -}
> -#define mk_pte mk_pte

Thanks!