[PATCH v1 17/29] mm/page_isolation: drop __folio_test_movable() check for large folios

David Hildenbrand posted 29 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v1 17/29] mm/page_isolation: drop __folio_test_movable() check for large folios
Posted by David Hildenbrand 3 months, 1 week ago
Currently, we only support migration of individual movable_ops pages, so
we can not run into that.

Reviewed-by: Zi Yan <ziy@nvidia.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 mm/page_isolation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index b97b965b3ed01..f72b6cd38b958 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -92,7 +92,7 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
 				h = size_to_hstate(folio_size(folio));
 				if (h && !hugepage_migration_supported(h))
 					return page;
-			} else if (!folio_test_lru(folio) && !__folio_test_movable(folio)) {
+			} else if (!folio_test_lru(folio)) {
 				return page;
 			}
 
-- 
2.49.0
Re: [PATCH v1 17/29] mm/page_isolation: drop __folio_test_movable() check for large folios
Posted by Harry Yoo 3 months, 1 week ago
On Mon, Jun 30, 2025 at 02:59:58PM +0200, David Hildenbrand wrote:
> Currently, we only support migration of individual movable_ops pages, so
> we can not run into that.
> 
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---

Looks correct to me.

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

-- 
Cheers,
Harry / Hyeonggon
Re: [PATCH v1 17/29] mm/page_isolation: drop __folio_test_movable() check for large folios
Posted by Lorenzo Stoakes 3 months, 1 week ago
On Mon, Jun 30, 2025 at 02:59:58PM +0200, David Hildenbrand wrote:
> Currently, we only support migration of individual movable_ops pages, so
> we can not run into that.
>
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Seems sensible, so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Maybe worth adding a VM_WARN_ON_ONCE() just in case? Or do you think not worth it?

> ---
>  mm/page_isolation.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index b97b965b3ed01..f72b6cd38b958 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -92,7 +92,7 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
>  				h = size_to_hstate(folio_size(folio));
>  				if (h && !hugepage_migration_supported(h))
>  					return page;
> -			} else if (!folio_test_lru(folio) && !__folio_test_movable(folio)) {
> +			} else if (!folio_test_lru(folio)) {
>  				return page;
>  			}
>
> --
> 2.49.0
>
Re: [PATCH v1 17/29] mm/page_isolation: drop __folio_test_movable() check for large folios
Posted by David Hildenbrand 3 months, 1 week ago
On 01.07.25 13:03, Lorenzo Stoakes wrote:
> On Mon, Jun 30, 2025 at 02:59:58PM +0200, David Hildenbrand wrote:
>> Currently, we only support migration of individual movable_ops pages, so
>> we can not run into that.
>>
>> Reviewed-by: Zi Yan <ziy@nvidia.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
> 
> Seems sensible, so:
> 
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> 
> Maybe worth adding a VM_WARN_ON_ONCE() just in case? Or do you think not worth it?

Not for now I think. Whoever wants to support compound pages has to 
fixup a bunch of other stuff first, before running into that one here.

So a full audit of all paths that handle page_has_movable_ops() is 
required either way.

-- 
Cheers,

David / dhildenb
Re: [PATCH v1 17/29] mm/page_isolation: drop __folio_test_movable() check for large folios
Posted by Harry Yoo 3 months, 1 week ago
On Tue, Jul 01, 2025 at 02:32:54PM +0200, David Hildenbrand wrote:
> On 01.07.25 13:03, Lorenzo Stoakes wrote:
> > On Mon, Jun 30, 2025 at 02:59:58PM +0200, David Hildenbrand wrote:
> > > Currently, we only support migration of individual movable_ops pages, so
> > > we can not run into that.
> > > 
> > > Reviewed-by: Zi Yan <ziy@nvidia.com>
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> > 
> > Seems sensible, so:
> > 
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > 
> > Maybe worth adding a VM_WARN_ON_ONCE() just in case? Or do you think not worth it?
> 
> Not for now I think. Whoever wants to support compound pages has to fixup a
> bunch of other stuff first, before running into that one here.
> 
> So a full audit of all paths that handle page_has_movable_ops() is required
> either way.

IIRC there was an RFC series last year [1] that adds support for
order > 0 pages in zsmalloc.

Cc'ing Barry and Tangquan in case it's still on their TODO list... 

[1] https://lore.kernel.org/linux-mm/20241121222521.83458-2-21cnbao@gmail.com

-- 
Cheers,
Harry / Hyeonggon