| Message ID | 20230821141728.2536317-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | secretmem: Conert page_is_secretmem() to folio_is_secretmem() | expand |
On Mon, Aug 21, 2023 at 03:17:28PM +0100, Matthew Wilcox (Oracle) wrote: > The only caller already has a folio, so use it to save calling > compound_head() in PageLRU() and remove a use of page->mapping. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org> > --- > include/linux/secretmem.h | 15 +++++++-------- > mm/gup.c | 2 +- > 2 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/include/linux/secretmem.h b/include/linux/secretmem.h > index 988528b5da43..6996f1f53f14 100644 > --- a/include/linux/secretmem.h > +++ b/include/linux/secretmem.h > @@ -6,24 +6,23 @@ > > extern const struct address_space_operations secretmem_aops; > > -static inline bool page_is_secretmem(struct page *page) > +static inline bool folio_is_secretmem(struct folio *folio) > { > struct address_space *mapping; > > /* > - * Using page_mapping() is quite slow because of the actual call > - * instruction and repeated compound_head(page) inside the > - * page_mapping() function. > + * Using folio_mapping() is quite slow because of the actual call > + * instruction. > * We know that secretmem pages are not compound and LRU so we can > * save a couple of cycles here. > */ > - if (PageCompound(page) || !PageLRU(page)) > + if (folio_test_large(folio) || folio_test_lru(folio)) > return false; > > mapping = (struct address_space *) > - ((unsigned long)page->mapping & ~PAGE_MAPPING_FLAGS); > + ((unsigned long)folio->mapping & ~PAGE_MAPPING_FLAGS); > > - if (!mapping || mapping != page->mapping) > + if (!mapping || mapping != folio->mapping) > return false; > > return mapping->a_ops == &secretmem_aops; > @@ -39,7 +38,7 @@ static inline bool vma_is_secretmem(struct vm_area_struct *vma) > return false; > } > > -static inline bool page_is_secretmem(struct page *page) > +static inline bool folio_is_secretmem(struct folio *folio) > { > return false; > } > diff --git a/mm/gup.c b/mm/gup.c > index 6d0c24e93425..2f8a2d89fde1 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -2600,7 +2600,7 @@ static int gup_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr, > if (!folio) > goto pte_unmap; > > - if (unlikely(page_is_secretmem(page))) { > + if (unlikely(folio_is_secretmem(folio))) { > gup_put_folio(folio, 1, flags); > goto pte_unmap; > } > -- > 2.40.1 > >
On 21.08.23 16:17, Matthew Wilcox (Oracle) wrote: > The only caller already has a folio, so use it to save calling > compound_head() in PageLRU() and remove a use of page->mapping. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > include/linux/secretmem.h | 15 +++++++-------- > mm/gup.c | 2 +- > 2 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/include/linux/secretmem.h b/include/linux/secretmem.h > index 988528b5da43..6996f1f53f14 100644 > --- a/include/linux/secretmem.h > +++ b/include/linux/secretmem.h > @@ -6,24 +6,23 @@ > > extern const struct address_space_operations secretmem_aops; > > -static inline bool page_is_secretmem(struct page *page) > +static inline bool folio_is_secretmem(struct folio *folio) > { > struct address_space *mapping; > > /* > - * Using page_mapping() is quite slow because of the actual call > - * instruction and repeated compound_head(page) inside the > - * page_mapping() function. > + * Using folio_mapping() is quite slow because of the actual call > + * instruction. > * We know that secretmem pages are not compound and LRU so we can > * save a couple of cycles here. > */ > - if (PageCompound(page) || !PageLRU(page)) > + if (folio_test_large(folio) || folio_test_lru(folio)) > return false; You converted a "!PageLRU(page)" into a "folio_test_lru(folio)". Shouldn't that be a "!folio_test_lru(folio)" ?
On Mon, Aug 21, 2023 at 05:32:50PM +0200, David Hildenbrand wrote: > > - if (PageCompound(page) || !PageLRU(page)) > > + if (folio_test_large(folio) || folio_test_lru(folio)) > > return false; > > You converted a "!PageLRU(page)" into a "folio_test_lru(folio)". > > Shouldn't that be a "!folio_test_lru(folio)" ? Well, that's embarrassing. Obviously I don't have any secretmem hardware, so my smoke-test didn't catch it. Thanks, will do a v2.
On 21.08.23 17:53, Matthew Wilcox wrote: > On Mon, Aug 21, 2023 at 05:32:50PM +0200, David Hildenbrand wrote: >>> - if (PageCompound(page) || !PageLRU(page)) >>> + if (folio_test_large(folio) || folio_test_lru(folio)) >>> return false; >> >> You converted a "!PageLRU(page)" into a "folio_test_lru(folio)". >> >> Shouldn't that be a "!folio_test_lru(folio)" ? > > Well, that's embarrassing. Obviously I don't have any secretmem > hardware, so my smoke-test didn't catch it. Thanks, will do a v2. > We do have a selftest: tools/testing/selftests/mm/memfd_secret.c I doubt it would have caught it, because ptrace doesn't use GUP-fast IIRC, so we always fail early in GUP code when doing the vma_is_secretmem() check. We'd explicitly have to call something that triggers GUP-fast (get_user_pages_fast/pin_user_pages_fast).
diff --git a/include/linux/secretmem.h b/include/linux/secretmem.h index 988528b5da43..6996f1f53f14 100644 --- a/include/linux/secretmem.h +++ b/include/linux/secretmem.h @@ -6,24 +6,23 @@ extern const struct address_space_operations secretmem_aops; -static inline bool page_is_secretmem(struct page *page) +static inline bool folio_is_secretmem(struct folio *folio) { struct address_space *mapping; /* - * Using page_mapping() is quite slow because of the actual call - * instruction and repeated compound_head(page) inside the - * page_mapping() function. + * Using folio_mapping() is quite slow because of the actual call + * instruction. * We know that secretmem pages are not compound and LRU so we can * save a couple of cycles here. */ - if (PageCompound(page) || !PageLRU(page)) + if (folio_test_large(folio) || folio_test_lru(folio)) return false; mapping = (struct address_space *) - ((unsigned long)page->mapping & ~PAGE_MAPPING_FLAGS); + ((unsigned long)folio->mapping & ~PAGE_MAPPING_FLAGS); - if (!mapping || mapping != page->mapping) + if (!mapping || mapping != folio->mapping) return false; return mapping->a_ops == &secretmem_aops; @@ -39,7 +38,7 @@ static inline bool vma_is_secretmem(struct vm_area_struct *vma) return false; } -static inline bool page_is_secretmem(struct page *page) +static inline bool folio_is_secretmem(struct folio *folio) { return false; } diff --git a/mm/gup.c b/mm/gup.c index 6d0c24e93425..2f8a2d89fde1 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -2600,7 +2600,7 @@ static int gup_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr, if (!folio) goto pte_unmap; - if (unlikely(page_is_secretmem(page))) { + if (unlikely(folio_is_secretmem(folio))) { gup_put_folio(folio, 1, flags); goto pte_unmap; }
The only caller already has a folio, so use it to save calling compound_head() in PageLRU() and remove a use of page->mapping. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/secretmem.h | 15 +++++++-------- mm/gup.c | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-)