| Message ID | f4dc7bb6-be3a-c1b-c30-37c4e0c16e4d@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mempolicy: cleanups leading to NUMA mpol without vma | expand |
On Mon, Sep 25, 2023 at 01:32:02AM -0700, Hugh Dickins wrote: > { > struct page *page = __alloc_pages(gfp | __GFP_COMP, order, > - preferred_nid, nodemask); > + preferred_nid, nodemask); I really prefer not to do this "align arguments with opening bracket" style. As long as they're indented enough to make them visually distinct from indentation-for-if-blocks, I find it annoying when functions get renamed to something with a different length and somebody then wastes time reindenting all the arguments to match. > + return page_rmappable_folio(page); I don't particularly object to the main thrust of this patch. I'm not sure I like it in huge_mm.h though. Maybe in mm/internal.h? I wouldn't want anyone outside mm/ calling it.
On Mon, 25 Sep 2023, Matthew Wilcox wrote: > On Mon, Sep 25, 2023 at 01:32:02AM -0700, Hugh Dickins wrote: > > { > > struct page *page = __alloc_pages(gfp | __GFP_COMP, order, > > - preferred_nid, nodemask); > > + preferred_nid, nodemask); > > I really prefer not to do this "align arguments with opening bracket" > style. As long as they're indented enough to make them visually distinct > from indentation-for-if-blocks, I find it annoying when functions get > renamed to something with a different length and somebody then wastes > time reindenting all the arguments to match. Okay, I don't care much about inserting spaces to align with the bracket, but didn't like those continuation args leftward of function name above. I'll adjust in v2, and eventually we reach a compromise. > > > + return page_rmappable_folio(page); > > I don't particularly object to the main thrust of this patch. I'm not > sure I like it in huge_mm.h though. Maybe in mm/internal.h? I > wouldn't want anyone outside mm/ calling it. I was expecting more resistance :) Right, I put it in huge_mm.h to be next to your folio_prep_large_rmappable() declaration, but it doesn't have to be there. Ooh, there's a folio_undo_large_rmappable() in mm/internal.h, I'll move page_rmappable_folio() next to that. Hugh
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index fa0350b0812a..58e7662a8a62 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -141,6 +141,15 @@ unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags); void folio_prep_large_rmappable(struct folio *folio); +static inline struct folio *page_rmappable_folio(struct page *page) +{ + struct folio *folio = (struct folio *)page; + + if (folio && folio_order(folio) > 1) + folio_prep_large_rmappable(folio); + return folio; +} + bool can_split_folio(struct folio *folio, int *pextra_pins); int split_huge_page_to_list(struct page *page, struct list_head *list); static inline int split_huge_page(struct page *page) @@ -281,6 +290,10 @@ static inline bool hugepage_vma_check(struct vm_area_struct *vma, } static inline void folio_prep_large_rmappable(struct folio *folio) {} +static inline struct folio *page_rmappable_folio(struct page *page) +{ + return (struct folio *)page; +} #define transparent_hugepage_flags 0UL diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 7ab6102d7da4..4c3b3f535630 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -2137,10 +2137,7 @@ struct folio *vma_alloc_folio(gfp_t gfp, int order, struct vm_area_struct *vma, mpol_cond_put(pol); gfp |= __GFP_COMP; page = alloc_page_interleave(gfp, order, nid); - folio = (struct folio *)page; - if (folio && order > 1) - folio_prep_large_rmappable(folio); - goto out; + return page_rmappable_folio(page); } if (pol->mode == MPOL_PREFERRED_MANY) { @@ -2150,10 +2147,7 @@ struct folio *vma_alloc_folio(gfp_t gfp, int order, struct vm_area_struct *vma, gfp |= __GFP_COMP; page = alloc_pages_preferred_many(gfp, order, node, pol); mpol_cond_put(pol); - folio = (struct folio *)page; - if (folio && order > 1) - folio_prep_large_rmappable(folio); - goto out; + return page_rmappable_folio(page); } if (unlikely(IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && hugepage)) { @@ -2247,12 +2241,7 @@ EXPORT_SYMBOL(alloc_pages); struct folio *folio_alloc(gfp_t gfp, unsigned order) { - struct page *page = alloc_pages(gfp | __GFP_COMP, order); - struct folio *folio = (struct folio *)page; - - if (folio && order > 1) - folio_prep_large_rmappable(folio); - return folio; + return page_rmappable_folio(alloc_pages(gfp | __GFP_COMP, order)); } EXPORT_SYMBOL(folio_alloc); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 95546f376302..5b1707d9025a 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4456,12 +4456,8 @@ struct folio *__folio_alloc(gfp_t gfp, unsigned int order, int preferred_nid, nodemask_t *nodemask) { struct page *page = __alloc_pages(gfp | __GFP_COMP, order, - preferred_nid, nodemask); - struct folio *folio = (struct folio *)page; - - if (folio && order > 1) - folio_prep_large_rmappable(folio); - return folio; + preferred_nid, nodemask); + return page_rmappable_folio(page); } EXPORT_SYMBOL(__folio_alloc);
folio_prep_large_rmappable() is being used repeatedly along with a conversion from page to folio, a check non-NULL, a check order > 1: wrap it all up into struct folio *page_rmappable_folio(struct page *). Signed-off-by: Hugh Dickins <hughd@google.com> --- include/linux/huge_mm.h | 13 +++++++++++++ mm/mempolicy.c | 17 +++-------------- mm/page_alloc.c | 8 ++------ 3 files changed, 18 insertions(+), 20 deletions(-)