diff mbox series

[13/13] mm: Convert swap_cluster_readahead and swap_vma_readahead to return a folio

Message ID 20231213215842.671461-14-willy@infradead.org (mailing list archive)
State New
Headers show
Series More swap folio conversions | expand

Commit Message

Matthew Wilcox Dec. 13, 2023, 9:58 p.m. UTC 
shmem_swapin_cluster() immediately converts the page back to a folio,
and swapin_readahead() may as well call folio_file_page() once instead
of having each function call it.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/shmem.c      |  8 +++-----
 mm/swap.h       |  6 +++---
 mm/swap_state.c | 21 ++++++++++-----------
 3 files changed, 16 insertions(+), 19 deletions(-)

Comments

Kairui Song Dec. 16, 2023, 1:58 p.m. UTC | #1 
Matthew Wilcox (Oracle) <willy@infradead.org> 于2023年12月14日周四 05:59写道：
>
> shmem_swapin_cluster() immediately converts the page back to a folio,
> and swapin_readahead() may as well call folio_file_page() once instead
> of having each function call it.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/shmem.c      |  8 +++-----
>  mm/swap.h       |  6 +++---
>  mm/swap_state.c | 21 ++++++++++-----------
>  3 files changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index c62f904ba1ca..a4d388973021 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1570,15 +1570,13 @@ static struct folio *shmem_swapin_cluster(swp_entry_t swap, gfp_t gfp,
>  {
>         struct mempolicy *mpol;
>         pgoff_t ilx;
> -       struct page *page;
> +       struct folio *folio;
>
>         mpol = shmem_get_pgoff_policy(info, index, 0, &ilx);
> -       page = swap_cluster_readahead(swap, gfp, mpol, ilx);
> +       folio = swap_cluster_readahead(swap, gfp, mpol, ilx);
>         mpol_cond_put(mpol);
>
> -       if (!page)
> -               return NULL;
> -       return page_folio(page);
> +       return folio;
>  }
>
>  /*
> diff --git a/mm/swap.h b/mm/swap.h
> index 82c68ccb5ab1..758c46ca671e 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -52,8 +52,8 @@ struct folio *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
>  struct folio *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_flags,
>                 struct mempolicy *mpol, pgoff_t ilx, bool *new_page_allocated,
>                 bool skip_if_exists);
> -struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
> -                                   struct mempolicy *mpol, pgoff_t ilx);
> +struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
> +               struct mempolicy *mpol, pgoff_t ilx);
>  struct page *swapin_readahead(swp_entry_t entry, gfp_t flag,
>                               struct vm_fault *vmf);
>
> @@ -80,7 +80,7 @@ static inline void show_swap_cache_info(void)
>  {
>  }
>
> -static inline struct page *swap_cluster_readahead(swp_entry_t entry,
> +static inline struct folio *swap_cluster_readahead(swp_entry_t entry,
>                         gfp_t gfp_mask, struct mempolicy *mpol, pgoff_t ilx)
>  {
>         return NULL;
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 1cb1d5d0583e..793b5b9e4f96 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -629,7 +629,7 @@ static unsigned long swapin_nr_pages(unsigned long offset)
>   * @mpol: NUMA memory allocation policy to be applied
>   * @ilx: NUMA interleave index, for use only when MPOL_INTERLEAVE
>   *
> - * Returns the struct page for entry and addr, after queueing swapin.
> + * Returns the struct folio for entry and addr, after queueing swapin.
>   *
>   * Primitive swap readahead code. We simply read an aligned block of
>   * (1 << page_cluster) entries in the swap area. This method is chosen
> @@ -640,7 +640,7 @@ static unsigned long swapin_nr_pages(unsigned long offset)
>   * are used for every page of the readahead: neighbouring pages on swap
>   * are fairly likely to have been swapped out from the same node.
>   */
> -struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
> +struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
>                                     struct mempolicy *mpol, pgoff_t ilx)
>  {
>         struct folio *folio;
> @@ -692,7 +692,7 @@ struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
>         if (unlikely(page_allocated))
>                 swap_read_folio(folio, false, NULL);
>         zswap_folio_swapin(folio);
> -       return folio_file_page(folio, swp_offset(entry));
> +       return folio;
>  }
>
>  int init_swap_address_space(unsigned int type, unsigned long nr_pages)
> @@ -796,7 +796,7 @@ static void swap_ra_info(struct vm_fault *vmf,
>   * @targ_ilx: NUMA interleave index, for use only when MPOL_INTERLEAVE
>   * @vmf: fault information
>   *
> - * Returns the struct page for entry and addr, after queueing swapin.
> + * Returns the struct folio for entry and addr, after queueing swapin.
>   *
>   * Primitive swap readahead code. We simply read in a few pages whose
>   * virtual addresses are around the fault address in the same vma.
> @@ -804,9 +804,8 @@ static void swap_ra_info(struct vm_fault *vmf,
>   * Caller must hold read mmap_lock if vmf->vma is not NULL.
>   *
>   */
> -static struct page *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
> -                                      struct mempolicy *mpol, pgoff_t targ_ilx,
> -                                      struct vm_fault *vmf)
> +static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
> +               struct mempolicy *mpol, pgoff_t targ_ilx, struct vm_fault *vmf)
>  {
>         struct blk_plug plug;
>         struct swap_iocb *splug = NULL;
> @@ -868,7 +867,7 @@ static struct page *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
>         if (unlikely(page_allocated))
>                 swap_read_folio(folio, false, NULL);
>         zswap_folio_swapin(folio);
> -       return folio_file_page(folio, swp_offset(entry));
> +       return folio;
>  }
>
>  /**
> @@ -888,14 +887,14 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
>  {
>         struct mempolicy *mpol;
>         pgoff_t ilx;
> -       struct page *page;
> +       struct folio *folio;
>
>         mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
> -       page = swap_use_vma_readahead() ?
> +       folio = swap_use_vma_readahead() ?
>                 swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
>                 swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
>         mpol_cond_put(mpol);
> -       return page;
> +       return folio_file_page(folio, swp_offset(entry));

Hi Matthew,

There is a bug here, folio could be NULL, and cause NULL dereference.
Matthew Wilcox Dec. 20, 2023, 12:54 a.m. UTC | #2 
On Sat, Dec 16, 2023 at 09:58:03PM +0800, Kairui Song wrote:
> > @@ -888,14 +887,14 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
> >  {
> >         struct mempolicy *mpol;
> >         pgoff_t ilx;
> > -       struct page *page;
> > +       struct folio *folio;
> >
> >         mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
> > -       page = swap_use_vma_readahead() ?
> > +       folio = swap_use_vma_readahead() ?
> >                 swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
> >                 swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
> >         mpol_cond_put(mpol);
> > -       return page;
> > +       return folio_file_page(folio, swp_offset(entry));
> 
> Hi Matthew,
> 
> There is a bug here, folio could be NULL, and cause NULL dereference.

Andrew, syzbot has also picked up on this.  Please add this -fix patch?

diff --git a/mm/swap_state.c b/mm/swap_state.c
index 793b5b9e4f96..8a3a8f1ab20a 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -894,6 +894,9 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
 		swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
 		swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
 	mpol_cond_put(mpol);
+
+	if (!folio)
+		return NULL;
 	return folio_file_page(folio, swp_offset(entry));
 }
diff mbox series

Patch

diff --git a/mm/shmem.c b/mm/shmem.c
index c62f904ba1ca..a4d388973021 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1570,15 +1570,13 @@  static struct folio *shmem_swapin_cluster(swp_entry_t swap, gfp_t gfp,
 {
 	struct mempolicy *mpol;
 	pgoff_t ilx;
-	struct page *page;
+	struct folio *folio;
 
 	mpol = shmem_get_pgoff_policy(info, index, 0, &ilx);
-	page = swap_cluster_readahead(swap, gfp, mpol, ilx);
+	folio = swap_cluster_readahead(swap, gfp, mpol, ilx);
 	mpol_cond_put(mpol);
 
-	if (!page)
-		return NULL;
-	return page_folio(page);
+	return folio;
 }
 
 /*
diff --git a/mm/swap.h b/mm/swap.h
index 82c68ccb5ab1..758c46ca671e 100644
--- a/mm/swap.h
+++ b/mm/swap.h
@@ -52,8 +52,8 @@  struct folio *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
 struct folio *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_flags,
 		struct mempolicy *mpol, pgoff_t ilx, bool *new_page_allocated,
 		bool skip_if_exists);
-struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
-				    struct mempolicy *mpol, pgoff_t ilx);
+struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
+		struct mempolicy *mpol, pgoff_t ilx);
 struct page *swapin_readahead(swp_entry_t entry, gfp_t flag,
 			      struct vm_fault *vmf);
 
@@ -80,7 +80,7 @@  static inline void show_swap_cache_info(void)
 {
 }
 
-static inline struct page *swap_cluster_readahead(swp_entry_t entry,
+static inline struct folio *swap_cluster_readahead(swp_entry_t entry,
 			gfp_t gfp_mask, struct mempolicy *mpol, pgoff_t ilx)
 {
 	return NULL;
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 1cb1d5d0583e..793b5b9e4f96 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -629,7 +629,7 @@  static unsigned long swapin_nr_pages(unsigned long offset)
  * @mpol: NUMA memory allocation policy to be applied
  * @ilx: NUMA interleave index, for use only when MPOL_INTERLEAVE
  *
- * Returns the struct page for entry and addr, after queueing swapin.
+ * Returns the struct folio for entry and addr, after queueing swapin.
  *
  * Primitive swap readahead code. We simply read an aligned block of
  * (1 << page_cluster) entries in the swap area. This method is chosen
@@ -640,7 +640,7 @@  static unsigned long swapin_nr_pages(unsigned long offset)
  * are used for every page of the readahead: neighbouring pages on swap
  * are fairly likely to have been swapped out from the same node.
  */
-struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
+struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
 				    struct mempolicy *mpol, pgoff_t ilx)
 {
 	struct folio *folio;
@@ -692,7 +692,7 @@  struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
 	if (unlikely(page_allocated))
 		swap_read_folio(folio, false, NULL);
 	zswap_folio_swapin(folio);
-	return folio_file_page(folio, swp_offset(entry));
+	return folio;
 }
 
 int init_swap_address_space(unsigned int type, unsigned long nr_pages)
@@ -796,7 +796,7 @@  static void swap_ra_info(struct vm_fault *vmf,
  * @targ_ilx: NUMA interleave index, for use only when MPOL_INTERLEAVE
  * @vmf: fault information
  *
- * Returns the struct page for entry and addr, after queueing swapin.
+ * Returns the struct folio for entry and addr, after queueing swapin.
  *
  * Primitive swap readahead code. We simply read in a few pages whose
  * virtual addresses are around the fault address in the same vma.
@@ -804,9 +804,8 @@  static void swap_ra_info(struct vm_fault *vmf,
  * Caller must hold read mmap_lock if vmf->vma is not NULL.
  *
  */
-static struct page *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
-				       struct mempolicy *mpol, pgoff_t targ_ilx,
-				       struct vm_fault *vmf)
+static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
+		struct mempolicy *mpol, pgoff_t targ_ilx, struct vm_fault *vmf)
 {
 	struct blk_plug plug;
 	struct swap_iocb *splug = NULL;
@@ -868,7 +867,7 @@  static struct page *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
 	if (unlikely(page_allocated))
 		swap_read_folio(folio, false, NULL);
 	zswap_folio_swapin(folio);
-	return folio_file_page(folio, swp_offset(entry));
+	return folio;
 }
 
 /**
@@ -888,14 +887,14 @@  struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
 {
 	struct mempolicy *mpol;
 	pgoff_t ilx;
-	struct page *page;
+	struct folio *folio;
 
 	mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
-	page = swap_use_vma_readahead() ?
+	folio = swap_use_vma_readahead() ?
 		swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
 		swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
 	mpol_cond_put(mpol);
-	return page;
+	return folio_file_page(folio, swp_offset(entry));
 }
 
 #ifdef CONFIG_SYSFS