| Message ID | 20240814154427.162475-5-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Provide a single definition of mk_pte() | expand |
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. We will investigate if that is really safe thing to do. Some people on vacation, so we might be not too quick with the response. > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > arch/s390/include/asm/pgtable.h | 11 ----------- > 1 file changed, 11 deletions(-) ... Thanks!
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!
diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h index 6a21d947a687..1bb7f33394d0 100644 --- a/arch/s390/include/asm/pgtable.h +++ b/arch/s390/include/asm/pgtable.h @@ -1413,17 +1413,6 @@ static inline pte_t mk_pte_phys(unsigned long physpage, pgprot_t pgprot) return pte_mkyoung(__pte); } -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 - #define pgd_index(address) (((address) >> PGDIR_SHIFT) & (PTRS_PER_PGD-1)) #define p4d_index(address) (((address) >> P4D_SHIFT) & (PTRS_PER_P4D-1)) #define pud_index(address) (((address) >> PUD_SHIFT) & (PTRS_PER_PUD-1))
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. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- arch/s390/include/asm/pgtable.h | 11 ----------- 1 file changed, 11 deletions(-)