On Mon, Jun 12, 2023 at 10:34:13PM +0800, Kefeng Wang wrote:
> @@ -959,7 +960,8 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> * Hugepage was successfully isolated and placed
> * on the cc->migratepages list.
> */
> - low_pfn += compound_nr(page) - 1;
> + folio = page_folio(page);
> + low_pfn += folio_nr_pages(folio) - 1;
> goto isolate_success_no_list;
Why is this safe? That is, how do we know that the folio can't be
dissolved under us at this point, then reallocated and hit the
VM_BUG_ON_PGFLAGS(PageTail(page), page) in folio_flags() when we
test folio_test_large()?
> @@ -1132,30 +1137,30 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> * and it's on LRU. It can only be a THP so the order
> * is safe to read and it's 0 for tail pages.
> */
^^^ This comment needs to be updated too.
> - mod_node_page_state(page_pgdat(page),
> - NR_ISOLATED_ANON + page_is_file_lru(page),
> - thp_nr_pages(page));
> + lruvec_del_folio(lruvec, folio);
> + mod_node_page_state(folio_pgdat(folio),
> + NR_ISOLATED_ANON + folio_is_file_lru(folio),
> + folio_nr_pages(folio));
node_stat_mod_folio()
On 2023/6/12 22:41, Matthew Wilcox wrote:
> On Mon, Jun 12, 2023 at 10:34:13PM +0800, Kefeng Wang wrote:
>> @@ -959,7 +960,8 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>> * Hugepage was successfully isolated and placed
>> * on the cc->migratepages list.
>> */
>> - low_pfn += compound_nr(page) - 1;
>> + folio = page_folio(page);
>> + low_pfn += folio_nr_pages(folio) - 1;
>> goto isolate_success_no_list;
>
> Why is this safe? That is, how do we know that the folio can't be
> dissolved under us at this point, then reallocated and hit the
> VM_BUG_ON_PGFLAGS(PageTail(page), page) in folio_flags() when we
> test folio_test_large()?
This is successfully isolated path, after isolate_hugetlb(), the folio
reference is incremented, so I think the folio can't be dissolved here,
correct me if I am wrong.
>
>> @@ -1132,30 +1137,30 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>> * and it's on LRU. It can only be a THP so the order
>> * is safe to read and it's 0 for tail pages.
>> */
>
> ^^^ This comment needs to be updated too.
will update
>
>> - mod_node_page_state(page_pgdat(page),
>> - NR_ISOLATED_ANON + page_is_file_lru(page),
>> - thp_nr_pages(page));
>> + lruvec_del_folio(lruvec, folio);
>> + mod_node_page_state(folio_pgdat(folio),
>> + NR_ISOLATED_ANON + folio_is_file_lru(folio),
>> + folio_nr_pages(folio));
>
> node_stat_mod_folio()
> ok
Thanks
>
On 2023/6/13 9:53, Kefeng Wang wrote:
>
>
> On 2023/6/12 22:41, Matthew Wilcox wrote:
>> On Mon, Jun 12, 2023 at 10:34:13PM +0800, Kefeng Wang wrote:
>>> @@ -959,7 +960,8 @@ isolate_migratepages_block(struct compact_control
>>> *cc, unsigned long low_pfn,
>>> * Hugepage was successfully isolated and placed
>>> * on the cc->migratepages list.
>>> */
>>> - low_pfn += compound_nr(page) - 1;
>>> + folio = page_folio(page);
>>> + low_pfn += folio_nr_pages(folio) - 1;
>>> goto isolate_success_no_list;
>>
>> Why is this safe? That is, how do we know that the folio can't be
>> dissolved under us at this point, then reallocated and hit the
>> VM_BUG_ON_PGFLAGS(PageTail(page), page) in folio_flags() when we
>> test folio_test_large()?
>
> This is successfully isolated path, after isolate_hugetlb(), the folio
> reference is incremented, so I think the folio can't be dissolved here,
> correct me if I am wrong.
Hi Matthew，is it enough for the above conversion from page to folio with
reference?