Received-SPF: pass (google.com: domain of linux-kernel+bounces-158739-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 25 Apr 2024 14:47:41 +0000
From: "Pankaj Raghav (Samsung)" <kernel@pankajraghav.com>
To: John Garry <john.g.garry@oracle.com>
Cc: axboe@kernel.dk, brauner@kernel.org, djwong@kernel.org,
	viro@zeniv.linux.org.uk, jack@suse.cz, akpm@linux-foundation.org,
	willy@infradead.org, dchinner@redhat.com, tytso@mit.edu, hch@lst.de,
	martin.petersen@oracle.com, nilay@linux.ibm.com,
	ritesh.list@gmail.com, mcgrof@kernel.org,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, ojaswin@linux.ibm.com, p.raghav@samsung.com,
	jbongio@google.com, okiselev@amazon.com
Subject: Re: [PATCH RFC 2/7] filemap: Change mapping_set_folio_min_order() ->
 mapping_set_folio_orders()
Message-ID: <20240425144741.houv6uoflhwmcc2u@quentin>
References: <20240422143923.3927601-1-john.g.garry@oracle.com>
 <20240422143923.3927601-3-john.g.garry@oracle.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240422143923.3927601-3-john.g.garry@oracle.com>

On Mon, Apr 22, 2024 at 02:39:18PM +0000, John Garry wrote:
> Borrowed from:
> 
> https://lore.kernel.org/linux-fsdevel/20240213093713.1753368-2-kernel@pankajraghav.com/
> (credit given in due course)
> 
> We will need to be able to only use a single folio order for buffered
> atomic writes, so allow the mapping folio order min and max be set.

> 
> We still have the restriction of not being able to support order-1
> folios - it will be required to lift this limit at some stage.

This is already supported upstream for file-backed folios:
commit: 8897277acfef7f70fdecc054073bea2542fc7a1b

> index fc8eb9c94e9c..c22455fa28a1 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -363,9 +363,10 @@ static inline void mapping_set_gfp_mask(struct address_space *m, gfp_t mask)
>  #endif
>  
>  /*
> - * mapping_set_folio_min_order() - Set the minimum folio order
> + * mapping_set_folio_orders() - Set the minimum and max folio order

In the new series (sorry forgot to CC you), I added a new helper called
mapping_set_folio_order_range() which does something similar to avoid
confusion based on willy's suggestion:
https://lore.kernel.org/linux-xfs/20240425113746.335530-3-kernel@pankajraghav.com/

mapping_set_folio_min_order() also sets max folio order to be 
MAX_PAGECACHE_ORDER order anyway. So no need of explicitly calling it
here?

>  /**
> @@ -400,7 +406,7 @@ static inline void mapping_set_folio_min_order(struct address_space *mapping,
>   */
>  static inline void mapping_set_large_folios(struct address_space *mapping)
>  {
> -	mapping_set_folio_min_order(mapping, 0);
> +	mapping_set_folio_orders(mapping, 0, MAX_PAGECACHE_ORDER);
>  }
>  
>  static inline unsigned int mapping_max_folio_order(struct address_space *mapping)
> diff --git a/mm/filemap.c b/mm/filemap.c
> index d81530b0aac0..d5effe50ddcb 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1898,9 +1898,15 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>  no_page:
>  	if (!folio && (fgp_flags & FGP_CREAT)) {
>  		unsigned int min_order = mapping_min_folio_order(mapping);
> -		unsigned int order = max(min_order, FGF_GET_ORDER(fgp_flags));
> +		unsigned int max_order = mapping_max_folio_order(mapping);
> +		unsigned int order = FGF_GET_ORDER(fgp_flags);
>  		int err;
>  
> +		if (order > max_order)
> +			order = max_order;
> +		else if (order < min_order)
> +			order = max_order;

order = min_order; ?
--
Pankaj