Re: [PATCH 01/28] mm: Remove folio_pincount_ptr() and head_compound_pincount()

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

 



On 1/11/23 06:28, Matthew Wilcox (Oracle) wrote:
We can use folio->_pincount directly, since all users are guarded by
tests of compound/large.

Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
---
  Documentation/core-api/pin_user_pages.rst | 29 +++++++++++------------
  include/linux/mm.h                        | 14 ++---------
  include/linux/mm_types.h                  |  5 ----
  mm/debug.c                                |  4 ++--
  mm/gup.c                                  |  8 +++----
  mm/huge_memory.c                          |  4 ++--
  mm/hugetlb.c                              |  4 ++--
  mm/page_alloc.c                           |  9 ++++---
  8 files changed, 32 insertions(+), 45 deletions(-)

Looks very nice, just a couple of questions about casts, below.

...

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4d9afa1048ea..d1e5ec875fd0 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -775,11 +775,13 @@ void free_compound_page(struct page *page)
static void prep_compound_head(struct page *page, unsigned int order)
  {
+	struct folio *folio = (struct folio *)page;

Casting, eh? I wonder if prep_compound_head() should just take a folio?
There are only a few callers of that.

+
  	set_compound_page_dtor(page, COMPOUND_PAGE_DTOR);
  	set_compound_order(page, order);
  	atomic_set(compound_mapcount_ptr(page), -1);
  	atomic_set(subpages_mapcount_ptr(page), 0);
-	atomic_set(compound_pincount_ptr(page), 0);
+	atomic_set(&folio->_pincount, 0);
  }
static void prep_compound_tail(struct page *head, int tail_idx)
@@ -1291,6 +1293,7 @@ static inline bool free_page_is_bad(struct page *page)
static int free_tail_pages_check(struct page *head_page, struct page *page)
  {
+	struct folio *folio = (struct folio *)head_page;

Similar to above: should the function just accept a folio instead of a
page?

Anyway, the patch as-is is good, and those points could be follow-up
changes if they are done, so either way, please feel free to add:


Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx>

thanks,
--
John Hubbard
NVIDIA




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux