madvise_inject_error skips pages

madvise_inject_error skips pages

[Matthew Wilcox]

I was going to send this patch:

+++ b/mm/madvise.c
@@ -1136,9 +1136,10 @@ static int madvise_inject_error(int behavior,
                /*
                 * When soft offlining hugepages, after migrating the page
                 * we dissolve it, therefore in the second loop "page" will
-                * no longer be a compound page.
+                * no longer be part of a large folio.
                 */
-               size = page_size(compound_head(page));
+               size = folio_size(page_folio(page));
+               start = start & ~(size - 1);

                if (behavior == MADV_SOFT_OFFLINE) {
                        pr_info("Soft offlining pfn %#lx at process virtual address %#lx\n",

because right now if you start in the middle of (e.g.) an order-4 folio
followed by an order-0 folio, you'll skip the order-0 folio immediately
following it.

But then I realised that we can come to this path in the middle of
a large file-backed folio that's mapped misaligned and has a COW page
in the middle, and the whole thing is just misguided.  So I gave up.
Anyone want to take a crack at fixing & testing this?

Re: madvise_inject_error skips pages

[Jane Chu]

Hi, Matthew,

On 7/16/2024 8:12 AM, Matthew Wilcox wrote:
> I was going to send this patch:
>
> +++ b/mm/madvise.c
> @@ -1136,9 +1136,10 @@ static int madvise_inject_error(int behavior,
>                  /*
>                   * When soft offlining hugepages, after migrating the page
>                   * we dissolve it, therefore in the second loop "page" will
> -                * no longer be a compound page.
> +                * no longer be part of a large folio.
>                   */
> -               size = page_size(compound_head(page));
> +               size = folio_size(page_folio(page));
> +               start = start & ~(size - 1);
>
>                  if (behavior == MADV_SOFT_OFFLINE) {
>                          pr_info("Soft offlining pfn %#lx at process virtual address %#lx\n",
>
> because right now if you start in the middle of (e.g.) an order-4 folio
> followed by an order-0 folio, you'll skip the order-0 folio immediately
> following it.
>
> But then I realised that we can come to this path in the middle of
> a large file-backed folio that's mapped misaligned and has a COW page
> in the middle, and the whole thing is just misguided.  So I gave up.
> Anyone want to take a crack at fixing & testing this?

Thanks for the report.

My understanding is, we should run folio_test_large() upfront, and treat 
both ends as special case

unless they're folio_size aligned, and iterate in folio_size for the 
middle.   I think this should work for

hugetlb and large folio in general, but I need to confirm this with tests.

Any thoughts?

BTW, unpoison does look right, I can address them together.

thanks,

-jane

Re: madvise_inject_error skips pages

[Miaohe Lin]

On 2024/7/17 5:33, Jane Chu wrote:
> Hi, Matthew,
> 
> On 7/16/2024 8:12 AM, Matthew Wilcox wrote:
>> I was going to send this patch:
>>
>> +++ b/mm/madvise.c
>> @@ -1136,9 +1136,10 @@ static int madvise_inject_error(int behavior,
>>                  /*
>>                   * When soft offlining hugepages, after migrating the page
>>                   * we dissolve it, therefore in the second loop "page" will
>> -                * no longer be a compound page.
>> +                * no longer be part of a large folio.
>>                   */
>> -               size = page_size(compound_head(page));
>> +               size = folio_size(page_folio(page));
>> +               start = start & ~(size - 1);
>>
>>                  if (behavior == MADV_SOFT_OFFLINE) {
>>                          pr_info("Soft offlining pfn %#lx at process virtual address %#lx\n",
>>
>> because right now if you start in the middle of (e.g.) an order-4 folio
>> followed by an order-0 folio, you'll skip the order-0 folio immediately
>> following it.
>>
>> But then I realised that we can come to this path in the middle of
>> a large file-backed folio that's mapped misaligned and has a COW page
>> in the middle, and the whole thing is just misguided.  So I gave up.
>> Anyone want to take a crack at fixing & testing this?
> 
> Thanks for the report.

Thanks both for your thoughts.

> 
> My understanding is, we should run folio_test_large() upfront, and treat both ends as special case
> 
> unless they're folio_size aligned, and iterate in folio_size for the middle.   I think this should work for
> 
> hugetlb and large folio in general, but I need to confirm this with tests.
> 
> Any thoughts?

I might be wrong but there might be some corner cases even if they're folio_size aligned.
For example, if pte[0] points to a pte mapped thp and pte[1..n] are cowed pages, these cowed
pages might be skipped in next loop because folio_size covers them but they doesn't belong to
this thp? Will it be better to decide @size upon pte entry?

Thanks.
.