* [PATCH] mm: huge_memory: convert split_huge_pages_all() to use a folio
@ 2022-12-29 12:25 Kefeng Wang
2022-12-29 23:28 ` Andrew Morton
2022-12-30 8:14 ` Matthew Wilcox
0 siblings, 2 replies; 5+ messages in thread
From: Kefeng Wang @ 2022-12-29 12:25 UTC (permalink / raw)
To: Andrew Morton, linux-mm; +Cc: linux-kernel, willy, Kefeng Wang
Straightforwardly convert split_huge_pages_all() to use a folio.
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
mm/huge_memory.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 266c4b557946..c8cbe7f62eaa 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2932,6 +2932,7 @@ static void split_huge_pages_all(void)
{
struct zone *zone;
struct page *page;
+ struct folio *folio;
unsigned long pfn, max_zone_pfn;
unsigned long total = 0, split = 0;
@@ -2944,24 +2945,32 @@ static void split_huge_pages_all(void)
int nr_pages;
page = pfn_to_online_page(pfn);
- if (!page || !get_page_unless_zero(page))
+ if (!page || PageTail(page))
+ continue;
+ folio = page_folio(page);
+ if (!folio_try_get(folio))
continue;
- if (zone != page_zone(page))
+ if (unlikely(page_folio(page) != folio))
+ goto next;
+
+ if (zone != folio_zone(folio))
goto next;
- if (!PageHead(page) || PageHuge(page) || !PageLRU(page))
+ if (!folio_test_large(folio)
+ || folio_test_hugetlb(folio)
+ || !folio_test_lru(folio))
goto next;
total++;
- lock_page(page);
- nr_pages = thp_nr_pages(page);
- if (!split_huge_page(page))
+ folio_lock(folio);
+ nr_pages = folio_nr_pages(folio);
+ if (!split_folio(folio))
split++;
pfn += nr_pages - 1;
- unlock_page(page);
+ folio_unlock(folio);
next:
- put_page(page);
+ folio_put(folio);
cond_resched();
}
}
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm: huge_memory: convert split_huge_pages_all() to use a folio
2022-12-29 12:25 [PATCH] mm: huge_memory: convert split_huge_pages_all() to use a folio Kefeng Wang
@ 2022-12-29 23:28 ` Andrew Morton
2022-12-30 1:36 ` Kefeng Wang
2022-12-30 8:14 ` Matthew Wilcox
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2022-12-29 23:28 UTC (permalink / raw)
To: Kefeng Wang; +Cc: linux-mm, linux-kernel, willy
On Thu, 29 Dec 2022 20:25:03 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> Straightforwardly convert split_huge_pages_all() to use a folio.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> mm/huge_memory.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 266c4b557946..c8cbe7f62eaa 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2932,6 +2932,7 @@ static void split_huge_pages_all(void)
> {
> struct zone *zone;
> struct page *page;
> + struct folio *folio;
> unsigned long pfn, max_zone_pfn;
> unsigned long total = 0, split = 0;
>
> @@ -2944,24 +2945,32 @@ static void split_huge_pages_all(void)
> int nr_pages;
>
> page = pfn_to_online_page(pfn);
> - if (!page || !get_page_unless_zero(page))
> + if (!page || PageTail(page))
> + continue;
Why is the PageTail() test added?
> + folio = page_folio(page);
> + if (!folio_try_get(folio))
> continue;
>
> - if (zone != page_zone(page))
> + if (unlikely(page_folio(page) != folio))
And this?
> + goto next;
> +
> + if (zone != folio_zone(folio))
> goto next;
>
> - if (!PageHead(page) || PageHuge(page) || !PageLRU(page))
> + if (!folio_test_large(folio)
> + || folio_test_hugetlb(folio)
> + || !folio_test_lru(folio))
> goto next;
>
> total++;
> - lock_page(page);
> - nr_pages = thp_nr_pages(page);
> - if (!split_huge_page(page))
> + folio_lock(folio);
> + nr_pages = folio_nr_pages(folio);
> + if (!split_folio(folio))
> split++;
> pfn += nr_pages - 1;
> - unlock_page(page);
> + folio_unlock(folio);
> next:
> - put_page(page);
> + folio_put(folio);
> cond_resched();
> }
> }
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm: huge_memory: convert split_huge_pages_all() to use a folio
2022-12-29 23:28 ` Andrew Morton
@ 2022-12-30 1:36 ` Kefeng Wang
0 siblings, 0 replies; 5+ messages in thread
From: Kefeng Wang @ 2022-12-30 1:36 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-mm, linux-kernel, willy
On 2022/12/30 7:28, Andrew Morton wrote:
> On Thu, 29 Dec 2022 20:25:03 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>
>> Straightforwardly convert split_huge_pages_all() to use a folio.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>> mm/huge_memory.c | 25 +++++++++++++++++--------
>> 1 file changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 266c4b557946..c8cbe7f62eaa 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2932,6 +2932,7 @@ static void split_huge_pages_all(void)
>> {
>> struct zone *zone;
>> struct page *page;
>> + struct folio *folio;
>> unsigned long pfn, max_zone_pfn;
>> unsigned long total = 0, split = 0;
>>
>> @@ -2944,24 +2945,32 @@ static void split_huge_pages_all(void)
>> int nr_pages;
>>
>> page = pfn_to_online_page(pfn);
>> - if (!page || !get_page_unless_zero(page))
>> + if (!page || PageTail(page))
>> + continue;
>
> Why is the PageTail() test added?
This function is trying to split huge pages, it traverse all the pfn,
the huge page already split when we met Head page, most importantly,
get_page_unless_zero() will do nothing on Tail page too.
>
>> + folio = page_folio(page);
>> + if (!folio_try_get(folio))
>> continue;
>>
>> - if (zone != page_zone(page))
>> + if (unlikely(page_folio(page) != folio))
>
> And this?
I think this is a double check in case of page is already changed by
someone else, suggested by Matthew[1]
[1]
https://lore.kernel.org/linux-mm/20221227195004.2809-1-sj@kernel.org/T/#m33047c152f6793bfebaa55cb1f4662fed73508d2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm: huge_memory: convert split_huge_pages_all() to use a folio
2022-12-29 12:25 [PATCH] mm: huge_memory: convert split_huge_pages_all() to use a folio Kefeng Wang
2022-12-29 23:28 ` Andrew Morton
@ 2022-12-30 8:14 ` Matthew Wilcox
2022-12-30 9:03 ` Kefeng Wang
1 sibling, 1 reply; 5+ messages in thread
From: Matthew Wilcox @ 2022-12-30 8:14 UTC (permalink / raw)
To: Kefeng Wang; +Cc: Andrew Morton, linux-mm, linux-kernel
On Thu, Dec 29, 2022 at 08:25:03PM +0800, Kefeng Wang wrote:
> - if (!PageHead(page) || PageHuge(page) || !PageLRU(page))
> + if (!folio_test_large(folio)
> + || folio_test_hugetlb(folio)
> + || !folio_test_lru(folio))
> goto next;
That is a completely illegible way of indenting this code! There's
no visual cue when the condition stops and when the next statement
begins. Try one of these:
if (!folio_test_large(folio) ||
folio_test_hugetlb(folio) ||
!folio_test_lru(folio))
goto next;
if (!folio_test_large(folio) ||
folio_test_hugetlb(folio) ||
!folio_test_lru(folio))
goto next;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm: huge_memory: convert split_huge_pages_all() to use a folio
2022-12-30 8:14 ` Matthew Wilcox
@ 2022-12-30 9:03 ` Kefeng Wang
0 siblings, 0 replies; 5+ messages in thread
From: Kefeng Wang @ 2022-12-30 9:03 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: Andrew Morton, linux-mm, linux-kernel
On 2022/12/30 16:14, Matthew Wilcox wrote:
> On Thu, Dec 29, 2022 at 08:25:03PM +0800, Kefeng Wang wrote:
>> - if (!PageHead(page) || PageHuge(page) || !PageLRU(page))
>> + if (!folio_test_large(folio)
>> + || folio_test_hugetlb(folio)
>> + || !folio_test_lru(folio))
>> goto next;
>
> That is a completely illegible way of indenting this code! There's
> no visual cue when the condition stops and when the next statement
> begins. Try one of these:
>
sorry about the bad indenting,
> if (!folio_test_large(folio) ||
> folio_test_hugetlb(folio) ||
> !folio_test_lru(folio))
> goto next;
>
will resend with this one, thanks for your review.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-30 9:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-29 12:25 [PATCH] mm: huge_memory: convert split_huge_pages_all() to use a folio Kefeng Wang
2022-12-29 23:28 ` Andrew Morton
2022-12-30 1:36 ` Kefeng Wang
2022-12-30 8:14 ` Matthew Wilcox
2022-12-30 9:03 ` Kefeng Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox