All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org,
	Wang Yugui <wangyugui@e16-tech.com>,
	Dave Chinner <david@fromorbit.com>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH v2 5/7] filemap: Allow __filemap_get_folio to allocate large folios
Date: Sun, 4 Jun 2023 11:09:25 -0700	[thread overview]
Message-ID: <20230604180925.GF72241@frogsfrogsfrogs> (raw)
In-Reply-To: <20230602222445.2284892-6-willy@infradead.org>

On Fri, Jun 02, 2023 at 11:24:42PM +0100, Matthew Wilcox (Oracle) wrote:
> Allow callers of __filemap_get_folio() to specify a preferred folio
> order in the FGP flags.  This is only honoured in the FGP_CREATE path;
> if there is already a folio in the page cache that covers the index,
> we will return it, no matter what its order is.  No create-around is
> attempted; we will only create folios which start at the specified index.
> Unmodified callers will continue to allocate order 0 folios.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  include/linux/pagemap.h | 23 ++++++++++++++++++++++
>  mm/filemap.c            | 42 ++++++++++++++++++++++++++++-------------
>  mm/readahead.c          | 13 -------------
>  3 files changed, 52 insertions(+), 26 deletions(-)
> 
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 7ab57a2bb576..667ce668f438 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -466,6 +466,19 @@ static inline void *detach_page_private(struct page *page)
>  	return folio_detach_private(page_folio(page));
>  }
>  
> +/*
> + * There are some parts of the kernel which assume that PMD entries
> + * are exactly HPAGE_PMD_ORDER.  Those should be fixed, but until then,
> + * limit the maximum allocation order to PMD size.  I'm not aware of any
> + * assumptions about maximum order if THP are disabled, but 8 seems like
> + * a good order (that's 1MB if you're using 4kB pages)
> + */
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +#define MAX_PAGECACHE_ORDER	HPAGE_PMD_ORDER
> +#else
> +#define MAX_PAGECACHE_ORDER	8
> +#endif
> +
>  #ifdef CONFIG_NUMA
>  struct folio *filemap_alloc_folio(gfp_t gfp, unsigned int order);
>  #else
> @@ -531,9 +544,19 @@ typedef unsigned int __bitwise fgp_t;
>  #define FGP_NOWAIT		((__force fgp_t)0x00000020)
>  #define FGP_FOR_MMAP		((__force fgp_t)0x00000040)
>  #define FGP_STABLE		((__force fgp_t)0x00000080)
> +#define FGP_GET_ORDER(fgp)	(((__force unsigned)fgp) >> 26)	/* top 6 bits */
>  
>  #define FGP_WRITEBEGIN		(FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE)
>  
> +static inline fgp_t fgp_set_order(size_t size)
> +{
> +	unsigned int shift = ilog2(size);
> +
> +	if (shift <= PAGE_SHIFT)
> +		return 0;
> +	return (__force fgp_t)((shift - PAGE_SHIFT) << 26);
> +}
> +
>  void *filemap_get_entry(struct address_space *mapping, pgoff_t index);
>  struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>  		fgp_t fgp_flags, gfp_t gfp);
> diff --git a/mm/filemap.c b/mm/filemap.c
> index eb89a815f2f8..10ea9321c36e 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1937,7 +1937,9 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>  		folio_wait_stable(folio);
>  no_page:
>  	if (!folio && (fgp_flags & FGP_CREAT)) {
> +		unsigned order = FGP_GET_ORDER(fgp_flags);
>  		int err;
> +
>  		if ((fgp_flags & FGP_WRITE) && mapping_can_writeback(mapping))
>  			gfp |= __GFP_WRITE;
>  		if (fgp_flags & FGP_NOFS)
> @@ -1946,26 +1948,40 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>  			gfp &= ~GFP_KERNEL;
>  			gfp |= GFP_NOWAIT | __GFP_NOWARN;
>  		}
> -
> -		folio = filemap_alloc_folio(gfp, 0);
> -		if (!folio)
> -			return ERR_PTR(-ENOMEM);
> -
>  		if (WARN_ON_ONCE(!(fgp_flags & (FGP_LOCK | FGP_FOR_MMAP))))
>  			fgp_flags |= FGP_LOCK;
>  
> -		/* Init accessed so avoid atomic mark_page_accessed later */
> -		if (fgp_flags & FGP_ACCESSED)
> -			__folio_set_referenced(folio);
> +		if (!mapping_large_folio_support(mapping))
> +			order = 0;
> +		if (order > MAX_PAGECACHE_ORDER)
> +			order = MAX_PAGECACHE_ORDER;
> +		/* If we're not aligned, allocate a smaller folio */
> +		if (index & ((1UL << order) - 1))
> +			order = __ffs(index);
>  
> -		err = filemap_add_folio(mapping, folio, index, gfp);
> -		if (unlikely(err)) {
> +		do {
> +			err = -ENOMEM;
> +			if (order == 1)
> +				order = 0;

Doesn't this interrupt the scale-down progression 2M -> 1M -> 512K ->
256K -> 128K -> 64K -> 32K -> 16K -> 4k?  What if I want 8k folios?

--D

> +			folio = filemap_alloc_folio(gfp, order);
> +			if (!folio)
> +				continue;
> +
> +			/* Init accessed so avoid atomic mark_page_accessed later */
> +			if (fgp_flags & FGP_ACCESSED)
> +				__folio_set_referenced(folio);
> +
> +			err = filemap_add_folio(mapping, folio, index, gfp);
> +			if (!err)
> +				break;
>  			folio_put(folio);
>  			folio = NULL;
> -			if (err == -EEXIST)
> -				goto repeat;
> -		}
> +		} while (order-- > 0);
>  
> +		if (err == -EEXIST)
> +			goto repeat;
> +		if (err)
> +			return ERR_PTR(err);
>  		/*
>  		 * filemap_add_folio locks the page, and for mmap
>  		 * we expect an unlocked page.
> diff --git a/mm/readahead.c b/mm/readahead.c
> index 47afbca1d122..59a071badb90 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -462,19 +462,6 @@ static int try_context_readahead(struct address_space *mapping,
>  	return 1;
>  }
>  
> -/*
> - * There are some parts of the kernel which assume that PMD entries
> - * are exactly HPAGE_PMD_ORDER.  Those should be fixed, but until then,
> - * limit the maximum allocation order to PMD size.  I'm not aware of any
> - * assumptions about maximum order if THP are disabled, but 8 seems like
> - * a good order (that's 1MB if you're using 4kB pages)
> - */
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -#define MAX_PAGECACHE_ORDER	HPAGE_PMD_ORDER
> -#else
> -#define MAX_PAGECACHE_ORDER	8
> -#endif
> -
>  static inline int ra_alloc_folio(struct readahead_control *ractl, pgoff_t index,
>  		pgoff_t mark, unsigned int order, gfp_t gfp)
>  {
> -- 
> 2.39.2
> 

  reply	other threads:[~2023-06-04 18:09 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-02 22:24 [PATCH v2 0/7] Create large folios in iomap buffered write path Matthew Wilcox (Oracle)
2023-06-02 22:24 ` [PATCH v2 1/7] iomap: Remove large folio handling in iomap_invalidate_folio() Matthew Wilcox (Oracle)
2023-06-04 17:58   ` Darrick J. Wong
2023-06-05  7:11   ` Christoph Hellwig
2023-06-02 22:24 ` [PATCH v2 2/7] doc: Correct the description of ->release_folio Matthew Wilcox (Oracle)
2023-06-04 17:55   ` Darrick J. Wong
2023-06-04 20:10     ` Matthew Wilcox
2023-06-04 20:33       ` Darrick J. Wong
2023-06-05 13:11         ` Matthew Wilcox
2023-06-05 15:07           ` Darrick J. Wong
2023-06-05  7:12   ` Christoph Hellwig
2023-06-02 22:24 ` [PATCH v2 3/7] iomap: Remove unnecessary test from iomap_release_folio() Matthew Wilcox (Oracle)
2023-06-04 18:01   ` Darrick J. Wong
2023-06-04 21:39     ` Matthew Wilcox
2023-06-05 21:10       ` Ritesh Harjani
2023-06-05  7:13   ` Christoph Hellwig
2023-06-02 22:24 ` [PATCH v2 4/7] filemap: Add fgp_t typedef Matthew Wilcox (Oracle)
2023-06-04 18:02   ` Darrick J. Wong
2023-06-05  7:14   ` Christoph Hellwig
2023-06-02 22:24 ` [PATCH v2 5/7] filemap: Allow __filemap_get_folio to allocate large folios Matthew Wilcox (Oracle)
2023-06-04 18:09   ` Darrick J. Wong [this message]
2023-06-04 21:48     ` Matthew Wilcox
2023-06-05 15:21       ` Darrick J. Wong
2023-06-05  7:16   ` Christoph Hellwig
2023-06-02 22:24 ` [PATCH v2 6/7] iomap: Create large folios in the buffered write path Matthew Wilcox (Oracle)
2023-06-04 18:10   ` Darrick J. Wong
2023-06-05  7:16   ` Christoph Hellwig
2023-06-02 22:24 ` [PATCH v2 7/7] iomap: Copy larger chunks from userspace Matthew Wilcox (Oracle)
2023-06-04 18:29   ` Darrick J. Wong
2023-06-04 22:11     ` Matthew Wilcox
2023-06-05  8:25       ` Yin, Fengwei
2023-06-06 18:07         ` Matthew Wilcox
2023-06-07  2:21           ` Yin Fengwei
2023-06-07  5:33             ` Yin, Fengwei
2023-06-07 15:55             ` Matthew Wilcox
2023-06-08  1:22               ` Yin Fengwei
2023-06-07  6:40           ` Yin Fengwei
2023-06-07 15:56             ` Matthew Wilcox
2023-06-04  0:19 ` [PATCH v2 0/7] Create large folios in iomap buffered write path Wang Yugui

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230604180925.GF72241@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=wangyugui@e16-tech.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.