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

[Jan Beulich <JBeulich@xxxxxxxx>]

On 29.07.2019 19:26, Andrew Cooper wrote:
> On 29/07/2019 16:48, Jan Beulich wrote:
>> On 29.07.2019 14:11, Andrew Cooper wrote:
>>> +    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).

domain_update_node_affinity() means to guarantee a non-empty mask (by
way of a similar assertion), when ->auto_node_affinity is set. Otoh
domain_set_node_affinity() may clear that flag, at which point I can't
see what would guarantee that the intersection would remain non-empty
as CPUs get offlined. (I don't understand, btw, why the function calls
domain_update_node_affinity() after clearing the flag.) Therefore I
don't think an assertion like you suggest would be legitimate. For
this there would be a prereq of domain_update_node_affinity() clearing
the flag when it finds the intersection has become empty. And even
then there would be a window in time where the intersection might be
actively empty, so some synchronization would be needed in addition.

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