mbox series

[v2,00/28] Convert GUP to folios

Message ID 20220110042406.499429-1-willy@infradead.org (mailing list archive)
Headers show
Series Convert GUP to folios | expand

Message

Matthew Wilcox Jan. 10, 2022, 4:23 a.m. UTC
This patch series is against my current folio for-next branch.  I know
it won't apply to sfr's next tree, and it's not for-next material yet.
I intend to submit it for 5.18 after I've rebased it to one of the
5.17-rc releases.

The overall effect of this (ignoring the primary "preparing for folios
that are not PAGE or PMD sized" purpose) is to reduce the size of gup.o
by ~700 bytes in the config I normally test with.

This patchset just converts existing implementations to use folios.
There's no new API for consumers here to provide information in a more
efficient (address, length) format.  That will be a separate patchset.

In v2, I've tried to address all the comments from the reviews I got
on v1.  Apologies if I missed anything; I got a lot of good feedback.
Primarily I separated out the folio changes (later) from the "While
I'm looking at this ..." changes (earlier).  I'm not sure the story
of the patchset is necessarily coherent this way, but it should be
easier to review.

Another big change is that compound_pincount is now available to all
compound pages, not just those that are order-2-or-higher.  Patch 11.

I did notice one bug in my original patchset which I'm disappointed GCC
didn't diagnose:

		pages[nr++] = nth_page(page, nr);

Given the massive reorg of the patchset, I didn't feel right using
anyone's SoB from v1 on any of the patches.  But, despite the increased
number of patches, I hope it's easier to review than v1.

And I'd dearly love a better name than 'folio_nth'.  page_nth() is
a temporary construct, so doesn't need a better name.  If you need
context, it's in the gup_folio_range_next() patch and its job
is to tell you, given a page and a folio, what # page it is within
a folio (so a number between [0 and folio_nr_pages())).

Matthew Wilcox (Oracle) (28):
  gup: Remove for_each_compound_range()
  gup: Remove for_each_compound_head()
  gup: Change the calling convention for compound_range_next()
  gup: Optimise compound_range_next()
  gup: Change the calling convention for compound_next()
  gup: Fix some contiguous memmap assumptions
  gup: Remove an assumption of a contiguous memmap
  gup: Handle page split race more efficiently
  gup: Turn hpage_pincount_add() into page_pincount_add()
  gup: Turn hpage_pincount_sub() into page_pincount_sub()
  mm: Make compound_pincount always available
  mm: Add folio_put_refs()
  mm: Add folio_pincount_ptr()
  mm: Convert page_maybe_dma_pinned() to use a folio
  gup: Add try_get_folio() and try_grab_folio()
  mm: Remove page_cache_add_speculative() and
    page_cache_get_speculative()
  gup: Add gup_put_folio()
  hugetlb: Use try_grab_folio() instead of try_grab_compound_head()
  gup: Convert try_grab_page() to call try_grab_folio()
  gup: Convert gup_pte_range() to use a folio
  gup: Convert gup_hugepte() to use a folio
  gup: Convert gup_huge_pmd() to use a folio
  gup: Convert gup_huge_pud() to use a folio
  gup: Convert gup_huge_pgd() to use a folio
  gup: Convert compound_next() to gup_folio_next()
  gup: Convert compound_range_next() to gup_folio_range_next()
  mm: Add isolate_lru_folio()
  gup: Convert check_and_migrate_movable_pages() to use a folio

 Documentation/core-api/pin_user_pages.rst |  18 +-
 arch/powerpc/include/asm/mmu_context.h    |   1 -
 include/linux/mm.h                        |  70 +++--
 include/linux/mm_types.h                  |  13 +-
 include/linux/pagemap.h                   |  11 -
 mm/debug.c                                |  14 +-
 mm/folio-compat.c                         |   8 +
 mm/gup.c                                  | 359 ++++++++++------------
 mm/hugetlb.c                              |   7 +-
 mm/internal.h                             |   8 +-
 mm/page_alloc.c                           |   3 +-
 mm/rmap.c                                 |   6 +-
 mm/vmscan.c                               |  43 ++-
 13 files changed, 263 insertions(+), 298 deletions(-)

Comments

Jason Gunthorpe Jan. 10, 2022, 3:31 p.m. UTC | #1
On Mon, Jan 10, 2022 at 04:23:38AM +0000, Matthew Wilcox (Oracle) wrote:
> This patch series is against my current folio for-next branch.  I know
> it won't apply to sfr's next tree, and it's not for-next material yet.
> I intend to submit it for 5.18 after I've rebased it to one of the
> 5.17-rc releases.
> 
> The overall effect of this (ignoring the primary "preparing for folios
> that are not PAGE or PMD sized" purpose) is to reduce the size of gup.o
> by ~700 bytes in the config I normally test with.
> 
> This patchset just converts existing implementations to use folios.
> There's no new API for consumers here to provide information in a more
> efficient (address, length) format.  That will be a separate patchset.
> 
> In v2, I've tried to address all the comments from the reviews I got
> on v1.  Apologies if I missed anything; I got a lot of good feedback.
> Primarily I separated out the folio changes (later) from the "While
> I'm looking at this ..." changes (earlier).  I'm not sure the story
> of the patchset is necessarily coherent this way, but it should be
> easier to review.
> 
> Another big change is that compound_pincount is now available to all
> compound pages, not just those that are order-2-or-higher.  Patch 11.
> 
> I did notice one bug in my original patchset which I'm disappointed GCC
> didn't diagnose:
> 
> 		pages[nr++] = nth_page(page, nr);
> 
> Given the massive reorg of the patchset, I didn't feel right using
> anyone's SoB from v1 on any of the patches.  But, despite the increased
> number of patches, I hope it's easier to review than v1.
> 
> And I'd dearly love a better name than 'folio_nth'.  page_nth() is
> a temporary construct, so doesn't need a better name.  If you need
> context, it's in the gup_folio_range_next() patch and its job
> is to tell you, given a page and a folio, what # page it is within
> a folio (so a number between [0 and folio_nr_pages())).

folio_tail_index() ?

Series still looks good to me, though I checked each patch
carefully than the prior series:

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason
Matthew Wilcox Jan. 10, 2022, 4:09 p.m. UTC | #2
On Mon, Jan 10, 2022 at 11:31:03AM -0400, Jason Gunthorpe wrote:
> 
> On Mon, Jan 10, 2022 at 04:23:38AM +0000, Matthew Wilcox (Oracle) wrote:
> > This patch series is against my current folio for-next branch.  I know
> > it won't apply to sfr's next tree, and it's not for-next material yet.
> > I intend to submit it for 5.18 after I've rebased it to one of the
> > 5.17-rc releases.
> > 
> > The overall effect of this (ignoring the primary "preparing for folios
> > that are not PAGE or PMD sized" purpose) is to reduce the size of gup.o
> > by ~700 bytes in the config I normally test with.
> > 
> > This patchset just converts existing implementations to use folios.
> > There's no new API for consumers here to provide information in a more
> > efficient (address, length) format.  That will be a separate patchset.
> > 
> > In v2, I've tried to address all the comments from the reviews I got
> > on v1.  Apologies if I missed anything; I got a lot of good feedback.
> > Primarily I separated out the folio changes (later) from the "While
> > I'm looking at this ..." changes (earlier).  I'm not sure the story
> > of the patchset is necessarily coherent this way, but it should be
> > easier to review.
> > 
> > Another big change is that compound_pincount is now available to all
> > compound pages, not just those that are order-2-or-higher.  Patch 11.
> > 
> > I did notice one bug in my original patchset which I'm disappointed GCC
> > didn't diagnose:
> > 
> > 		pages[nr++] = nth_page(page, nr);
> > 
> > Given the massive reorg of the patchset, I didn't feel right using
> > anyone's SoB from v1 on any of the patches.  But, despite the increased
> > number of patches, I hope it's easier to review than v1.
> > 
> > And I'd dearly love a better name than 'folio_nth'.  page_nth() is
> > a temporary construct, so doesn't need a better name.  If you need
> > context, it's in the gup_folio_range_next() patch and its job
> > is to tell you, given a page and a folio, what # page it is within
> > a folio (so a number between [0 and folio_nr_pages())).
> 
> folio_tail_index() ?

I'm a little wary of "index" because folios are used to cache file data,
and folio->index means offset of the folio within the file.  We could
make the argument for folio_page_index() (since it might be the head
page, not a tail page), and argue this is the index into the folio,
not the index into the file.

It's better than folio_nth ;-)

> Series still looks good to me, though I checked each patch
> carefully than the prior series:
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Thanks!
William Kucharski Jan. 10, 2022, 5:26 p.m. UTC | #3
Series still looks good.

Reviewed-by: William Kucharski <william.kucharski@oracle.com>

> On Jan 9, 2022, at 9:23 PM, Matthew Wilcox (Oracle) <willy@infradead.org> wrote:
> 
> This patch series is against my current folio for-next branch.  I know
> it won't apply to sfr's next tree, and it's not for-next material yet.
> I intend to submit it for 5.18 after I've rebased it to one of the
> 5.17-rc releases.
> 
> The overall effect of this (ignoring the primary "preparing for folios
> that are not PAGE or PMD sized" purpose) is to reduce the size of gup.o
> by ~700 bytes in the config I normally test with.
> 
> This patchset just converts existing implementations to use folios.
> There's no new API for consumers here to provide information in a more
> efficient (address, length) format.  That will be a separate patchset.
> 
> In v2, I've tried to address all the comments from the reviews I got
> on v1.  Apologies if I missed anything; I got a lot of good feedback.
> Primarily I separated out the folio changes (later) from the "While
> I'm looking at this ..." changes (earlier).  I'm not sure the story
> of the patchset is necessarily coherent this way, but it should be
> easier to review.
> 
> Another big change is that compound_pincount is now available to all
> compound pages, not just those that are order-2-or-higher.  Patch 11.
> 
> I did notice one bug in my original patchset which I'm disappointed GCC
> didn't diagnose:
> 
> 		pages[nr++] = nth_page(page, nr);
> 
> Given the massive reorg of the patchset, I didn't feel right using
> anyone's SoB from v1 on any of the patches.  But, despite the increased
> number of patches, I hope it's easier to review than v1.
> 
> And I'd dearly love a better name than 'folio_nth'.  page_nth() is
> a temporary construct, so doesn't need a better name.  If you need
> context, it's in the gup_folio_range_next() patch and its job
> is to tell you, given a page and a folio, what # page it is within
> a folio (so a number between [0 and folio_nr_pages())).
> 
> Matthew Wilcox (Oracle) (28):
>  gup: Remove for_each_compound_range()
>  gup: Remove for_each_compound_head()
>  gup: Change the calling convention for compound_range_next()
>  gup: Optimise compound_range_next()
>  gup: Change the calling convention for compound_next()
>  gup: Fix some contiguous memmap assumptions
>  gup: Remove an assumption of a contiguous memmap
>  gup: Handle page split race more efficiently
>  gup: Turn hpage_pincount_add() into page_pincount_add()
>  gup: Turn hpage_pincount_sub() into page_pincount_sub()
>  mm: Make compound_pincount always available
>  mm: Add folio_put_refs()
>  mm: Add folio_pincount_ptr()
>  mm: Convert page_maybe_dma_pinned() to use a folio
>  gup: Add try_get_folio() and try_grab_folio()
>  mm: Remove page_cache_add_speculative() and
>    page_cache_get_speculative()
>  gup: Add gup_put_folio()
>  hugetlb: Use try_grab_folio() instead of try_grab_compound_head()
>  gup: Convert try_grab_page() to call try_grab_folio()
>  gup: Convert gup_pte_range() to use a folio
>  gup: Convert gup_hugepte() to use a folio
>  gup: Convert gup_huge_pmd() to use a folio
>  gup: Convert gup_huge_pud() to use a folio
>  gup: Convert gup_huge_pgd() to use a folio
>  gup: Convert compound_next() to gup_folio_next()
>  gup: Convert compound_range_next() to gup_folio_range_next()
>  mm: Add isolate_lru_folio()
>  gup: Convert check_and_migrate_movable_pages() to use a folio
> 
> Documentation/core-api/pin_user_pages.rst |  18 +-
> arch/powerpc/include/asm/mmu_context.h    |   1 -
> include/linux/mm.h                        |  70 +++--
> include/linux/mm_types.h                  |  13 +-
> include/linux/pagemap.h                   |  11 -
> mm/debug.c                                |  14 +-
> mm/folio-compat.c                         |   8 +
> mm/gup.c                                  | 359 ++++++++++------------
> mm/hugetlb.c                              |   7 +-
> mm/internal.h                             |   8 +-
> mm/page_alloc.c                           |   3 +-
> mm/rmap.c                                 |   6 +-
> mm/vmscan.c                               |  43 ++-
> 13 files changed, 263 insertions(+), 298 deletions(-)
> 
> -- 
> 2.33.0
>