diff mbox series

[RFC,03/23] filemap: add folio with at least mapping_min_order in __filemap_get_folio

Message ID 20230915183848.1018717-4-kernel@pankajraghav.com (mailing list archive)
State New
Headers show
Series Enable block size > page size in XFS | expand

Commit Message

Pankaj Raghav (Samsung) Sept. 15, 2023, 6:38 p.m. UTC 
From: Pankaj Raghav <p.raghav@samsung.com>

__filemap_get_folio() with FGP_CREAT should allocate at least folio of
filemap's min_order set using folio_set_mapping_orders().

A higher order folio than min_order by definition is a multiple of the
min_order. If an index is aligned to an order higher than a min_order, it
will also be aligned to the min order.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 mm/filemap.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Matthew Wilcox Sept. 15, 2023, 7 p.m. UTC | #1 
On Fri, Sep 15, 2023 at 08:38:28PM +0200, Pankaj Raghav wrote:
> +++ b/mm/filemap.c
> @@ -1862,6 +1862,10 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>  		fgf_t fgp_flags, gfp_t gfp)
>  {
>  	struct folio *folio;
> +	int min_order = mapping_min_folio_order(mapping);
> +	int nr_of_pages = (1U << min_order);
> +
> +	index = round_down(index, nr_of_pages);
>  
>  repeat:
>  	folio = filemap_get_entry(mapping, index);
> @@ -1929,8 +1933,14 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>  			err = -ENOMEM;
>  			if (order == 1)
>  				order = 0;
> +			if (order < min_order)
> +				order = min_order;

... oh, you do something similar here to what I recommend in my previous
response.  I don't understand why you need the previous patch.

> +			if (min_order)
> +				VM_BUG_ON(index & ((1UL << order) - 1));

You don't need the 'if' here; index & ((1 << 0) - 1) becomes false.
Pankaj Raghav Sept. 20, 2023, 8:06 a.m. UTC | #2 
On 2023-09-15 21:00, Matthew Wilcox wrote:
> On Fri, Sep 15, 2023 at 08:38:28PM +0200, Pankaj Raghav wrote:
>> +++ b/mm/filemap.c
>> @@ -1862,6 +1862,10 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>>  		fgf_t fgp_flags, gfp_t gfp)
>>  {
>>  	struct folio *folio;
>> +	int min_order = mapping_min_folio_order(mapping);
>> +	int nr_of_pages = (1U << min_order);
>> +
>> +	index = round_down(index, nr_of_pages);
>>  
>>  repeat:
>>  	folio = filemap_get_entry(mapping, index);
>> @@ -1929,8 +1933,14 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
>>  			err = -ENOMEM;
>>  			if (order == 1)
>>  				order = 0;
>> +			if (order < min_order)
>> +				order = min_order;
> 
> ... oh, you do something similar here to what I recommend in my previous
> response.  I don't understand why you need the previous patch.
> 

Hmm, we made changes here a bit later and that is why it is a duplicated
I guess in both iomap fgf order and clamping the order here to min_order. We could
remove the previous patch and retain this one here.

>> +			if (min_order)
>> +				VM_BUG_ON(index & ((1UL << order) - 1));
> 
> You don't need the 'if' here; index & ((1 << 0) - 1) becomes false.
> 

Sounds good!
diff mbox series

Patch

diff --git a/mm/filemap.c b/mm/filemap.c
index 8962d1255905..b1ce63143df5 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1862,6 +1862,10 @@  struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
 		fgf_t fgp_flags, gfp_t gfp)
 {
 	struct folio *folio;
+	int min_order = mapping_min_folio_order(mapping);
+	int nr_of_pages = (1U << min_order);
+
+	index = round_down(index, nr_of_pages);
 
 repeat:
 	folio = filemap_get_entry(mapping, index);
@@ -1929,8 +1933,14 @@  struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
 			err = -ENOMEM;
 			if (order == 1)
 				order = 0;
+			if (order < min_order)
+				order = min_order;
 			if (order > 0)
 				alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN;
+
+			if (min_order)
+				VM_BUG_ON(index & ((1UL << order) - 1));
+
 			folio = filemap_alloc_folio(alloc_gfp, order);
 			if (!folio)
 				continue;
@@ -1944,7 +1954,7 @@  struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
 				break;
 			folio_put(folio);
 			folio = NULL;
-		} while (order-- > 0);
+		} while (order-- > min_order);
 
 		if (err == -EEXIST)
 			goto repeat;