2023-09-16 03:03:42

by Pankaj Raghav (Samsung)

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

From: Pankaj Raghav <[email protected]>

__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 <[email protected]>
---
mm/filemap.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

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;
--
2.40.1


2023-09-16 06:46:22

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [RFC 03/23] filemap: add folio with at least mapping_min_order in __filemap_get_folio

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.

2023-09-20 23:28:10

by Pankaj Raghav

[permalink] [raw]
Subject: Re: [RFC 03/23] filemap: add folio with at least mapping_min_order in __filemap_get_folio

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!