Re: [PATCH v2 1/3] xen/sched: introduce cpupool_update_node_affinity()

[Juergen Gross <jgross@xxxxxxxx>]

On 15.08.22 13:41, Jan Beulich wrote:
> On 15.08.2022 13:04, Juergen Gross wrote:
> > For updating the node affinities of all domains in a cpupool add a new
> > function cpupool_update_node_affinity().
> >
> > In order to avoid multiple allocations of cpumasks carve out memory
> > allocation and freeing from domain_update_node_affinity() into new
> > helpers, which can be used by cpupool_update_node_affinity().
> >
> > Modify domain_update_node_affinity() to take an additional parameter
> > for passing the allocated memory in and to allocate and free the memory
> > via the new helpers in case NULL was passed.
> >
> > This will help later to pre-allocate the cpumasks in order to avoid
> > allocations in stop-machine context.
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> with the observation that ...
>
> > --- a/xen/common/sched/core.c
> > +++ b/xen/common/sched/core.c
> > @@ -1824,9 +1824,28 @@ int vcpu_affinity_domctl(struct domain *d, uint32_t cmd,
> >       return ret;
> >   }
> >   
> > -void domain_update_node_affinity(struct domain *d)
> > +bool update_node_aff_alloc(struct affinity_masks *affinity)
> >   {
> > -    cpumask_var_t dom_cpumask, dom_cpumask_soft;
> > +    if ( !alloc_cpumask_var(&affinity->hard) )
> > +        return false;
> > +    if ( !alloc_cpumask_var(&affinity->soft) )
> > +    {
> > +        free_cpumask_var(affinity->hard);
> > +        return false;
> > +    }
> > +
> > +    return true;
> > +}
> > +
> > +void update_node_aff_free(struct affinity_masks *affinity)
> > +{
> > +    free_cpumask_var(affinity->soft);
> > +    free_cpumask_var(affinity->hard);
> > +}
> > +
> > +void domain_update_node_aff(struct domain *d, struct affinity_masks *affinity)
> > +{
> > +    struct affinity_masks masks = { };
> ... the initializer doesn't really look to be needed here, just like
> you don't have one in cpupool_update_node_affinity(). The one thing
> I'm not sure about is whether old gcc might mis-report a potentially
> uninitialized variable with the initializer dropped ...
Hmm, yes, I think the initializer was needed only in V1.

I guess you could remove it while committing in case no respin of the
series is needed otherwise?


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature