2024-06-04 08:26:08

by yangge1116

Subject: [PATCH] mm/gup: don't check page lru flag before draining it

From: yangge <[email protected]>

If a page is added in pagevec, its ref count increases one, remove
the page from pagevec decreases one. Page migration requires the
page is not referrened by others except page mapping. Before
migrating a page, we should try to drain the page from pagevec in
case the page is in it, however, folio_test_lru() is not sufficient
to tell whether the page is in pagevec or not, if the page is in
pagevec, the migration will fail.

Remove the condition and drain lru once to ensure the page is not
referrenced by pagevec.

Signed-off-by: yangge <[email protected]>
---
mm/gup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/gup.c b/mm/gup.c
index ca0f5ce..890dcbc 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2411,7 +2411,7 @@ static unsigned long collect_longterm_unpinnable_pages(
continue;
}

- if (!folio_test_lru(folio) && drain_allow) {
+ if (drain_allow) {
lru_add_drain_all();
drain_allow = false;
}
--
2.7.4

2024-06-04 09:05:15

by Baolin Wang

Subject: Re: [PATCH] mm/gup: don't check page lru flag before draining it

On 2024/6/4 16:09, [email protected] wrote:
> From: yangge <[email protected]>
>
> If a page is added in pagevec, its ref count increases one, remove
> the page from pagevec decreases one. Page migration requires the
> page is not referrened by others except page mapping. Before
> migrating a page, we should try to drain the page from pagevec in
> case the page is in it, however, folio_test_lru() is not sufficient
> to tell whether the page is in pagevec or not, if the page is in
> pagevec, the migration will fail.
>
> Remove the condition and drain lru once to ensure the page is not
> referrenced by pagevec.

This looks sane to me and seems a simple way to fix.

> Signed-off-by: yangge <[email protected]>
> ---
> mm/gup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index ca0f5ce..890dcbc 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2411,7 +2411,7 @@ static unsigned long collect_longterm_unpinnable_pages(
> continue;
> }
>
> - if (!folio_test_lru(folio) && drain_allow) {
> + if (drain_allow) {
> lru_add_drain_all();
> drain_allow = false;
> }

You should rebase your code on the latest mm-unstable branch, as
collect_longterm_unpinnable_pages() in the upstream has been replaced
with collect_longterm_unpinnable_folios().

2024-06-04 09:28:22

by yangge1116

Subject: Re: [PATCH] mm/gup: don't check page lru flag before draining it

在 2024/6/4 下午4:56, Baolin Wang 写道:
>
>
> On 2024/6/4 16:09, [email protected] wrote:
>> From: yangge <[email protected]>
>>
>> If a page is added in pagevec, its ref count increases one, remove
>> the page from pagevec decreases one. Page migration requires the
>> page is not referrened by others except page mapping. Before
>> migrating a page, we should try to drain the page from pagevec in
>> case the page is in it, however, folio_test_lru() is not sufficient
>> to tell whether the page is in pagevec or not, if the page is in
>> pagevec, the migration will fail.
>>
>> Remove the condition and drain lru once to ensure the page is not
>> referrenced by pagevec.
>
> This looks sane to me and seems a simple way to fix.
>
>> Signed-off-by: yangge <[email protected]>
>> ---
>>   mm/gup.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/gup.c b/mm/gup.c
>> index ca0f5ce..890dcbc 100644
>> --- a/mm/gup.c
>> +++ b/mm/gup.c
>> @@ -2411,7 +2411,7 @@ static unsigned long
>> collect_longterm_unpinnable_pages(
>>               continue;
>>           }
>> -        if (!folio_test_lru(folio) && drain_allow) {
>> +        if (drain_allow) {
>>               lru_add_drain_all();
>>               drain_allow = false;
>>           }
>
> You should rebase your code on the latest mm-unstable branch, as
> collect_longterm_unpinnable_pages() in the upstream has been replaced
> with collect_longterm_unpinnable_folios().
Ok, thanks