Re: [Xen-devel] [PATCH v3 01/10] page-alloc: Clamp get_free_buddy() to online nodes

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/07/2019 16:48, Jan Beulich wrote:
> On 29.07.2019 14:11, Andrew Cooper wrote:
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -811,11 +811,18 @@ static struct page_info *get_free_buddy(unsigned int 
>> zone_lo,
>>                                           const struct domain *d)
>>   {
>>       nodeid_t first, node = MEMF_get_node(memflags), req_node = node;
>> -    nodemask_t nodemask = d ? d->node_affinity : node_online_map;
>> +    nodemask_t nodemask = node_online_map;
>>       unsigned int j, zone, nodemask_retry = 0;
>>       struct page_info *pg;
>>       bool use_unscrubbed = (memflags & MEMF_no_scrub);
>>   
>> +    /*
>> +     * d->node_affinity is our preferred allocation set if provided, but it
>> +     * may have bit set outside of node_online_map.  Clamp it.
> Would you mind adding the apparently missing "s" to "bit"?

Oops yes.

>
>> +     */
>> +    if ( d )
>> +        nodes_and(nodemask, nodemask, d->node_affinity);
> Despite my earlier ack: Code further down assumes a non-empty mask,
> which is no longer guaranteed afaics.

Nothing previous guaranteed that d->node_affinity had any bits set in
it, either.

That said, in practice it is either ALL, or something derived from the
cpu=>node mappings, so I don't think this is a problem in practice.

> I think you want to append an
> "intersects" check in the if().

I think it would be better to assert that callers don't give us complete
junk.

> With that feel free to promote my
> A-b to R-b.

How about:

    if ( d )
    {
        if ( nodes_intersect(nodemask, d->node_affinity) )
            nodes_and(nodemask, nodemask, d->node_affinity);
        else
            ASSERT_UNREACHABLE();
    }

?

This change has passed my normal set of prepush checks (not not that
there is anything interesting NUMA-wise in there).

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel