Re: [Devel] [PATCH hci-8.1 v2 1/2] mm/memcontrol: allocate numa_migrate targets of the source folio order

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: memcg numa_migrate uses no-reclaim THISNODE gfp for all large folio orders
--

commit dc423622ee7bf287039eede1edeca15b40cb3108
Author: Konstantin Khorenko <khorenko@virtuozzo.com>

mm/memcontrol: allocate numa_migrate targets of the source folio order

This patch makes memcg_numa_migrate_new_folio() allocate the destination
folio with the same order as the source large folio, instead of always
using HPAGE_PMD_ORDER. It also drops the thp_migration_supported() check.

> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 0734c0fa21d05..d8a87913ace37 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c

[ ... ]

> @@ -3487,13 +3488,13 @@ static struct folio *memcg_numa_migrate_new_folio(struct folio *old_folio,
>  		VM_BUG_ON(ms->current_node >= MAX_NUMNODES);
>  	}
>  
> -	if (thp_migration_supported() && folio_test_large(old_folio)) {
> -		return __folio_alloc(GFP_TRANSHUGE_LIGHT | __GFP_THISNODE,
> -				HPAGE_PMD_ORDER, ms->current_node,
> -				ms->target_nodes);
> +	if (folio_test_large(old_folio)) {
> +		gfp_mask = GFP_TRANSHUGE_LIGHT | __GFP_THISNODE;
> +		order = folio_order(old_folio);
>  	}

[Severity: Medium]
The order fix looks right, but the no-reclaim, single-node policy is now
applied to every large folio, not only PMD-sized THPs.

GFP_TRANSHUGE_LIGHT has no __GFP_DIRECT_RECLAIM, and __GFP_THISNODE pins
the allocation to ms->current_node. For a small-order file folio or an
mTHP, can the allocation now fail on a fragmented node, with no fallback
to the other nodes in target_nodes? Before this patch, such folios got an
order-0 destination allocated with GFP_HIGHUSER_MOVABLE | __GFP_NORETRY,
which does allow direct reclaim.

If the allocation fails, migrate_folio_unmap() returns -ENOMEM. In
migrate_pages_batch() the -ENOMEM case then does this:

    case -ENOMEM:
        ...
        if (is_large && !nosplit) {
            int ret = try_split_folio(folio, split_folios, mode);
            ...
        }
        ...
        rc_saved = rc;
        if (list_empty(&unmap_folios))
            goto out;

When the split also fails (for example -EAGAIN from an elevated refcount,
or a mapping whose minimum folio order is above 0), migrate_pages()
returns a negative value. Then in __memcg_numa_migrate_pages():

    ret = migrate_pages(&pages, memcg_numa_migrate_new_folio, ...);
    putback_movable_pages(&pages);
    if (ret < 0)
        return ret;

Does this abort the whole memory.numa_migrate write with -ENOMEM and
leave the remaining folios of the memcg unmigrated?

The old PMD-THP path had the same failure semantics. This patch widens
it to all large folio orders, and the commit message doesn't mention the
change in allocation policy.

Would it be reasonable to fall back to order 0 or to a reclaim-capable
gfp mask when the large allocation fails?

>  
> -	return __folio_alloc(gfp_mask, 0, ms->current_node, ms->target_nodes);
> +	return __folio_alloc(gfp_mask, order, ms->current_node,
> +			     ms->target_nodes);
>  }

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261028-b4-prep-khorenko-vstor-151607-numa-migrate-order-v2-0-b973a82a5b0c@virtuozzo.com?part=1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel