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
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
