Re: [PATCH] xen/domain: Factor domlist_{insert,remove}() out of domain_{create,destroy}()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.07.2024 14:04, Andrew Cooper wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -64,6 +64,57 @@ DEFINE_RCU_READ_LOCK(domlist_read_lock);
>  static struct domain *domain_hash[DOMAIN_HASH_SIZE];
>  struct domain *domain_list;
>  
> +/*
> + * Insert a domain into the domlist/hash.  This allows the domain to be 
> looked
> + * up by domid, and therefore to be the subject of hypercalls/etc.
> + */
> +static void domlist_insert(struct domain *d)
> +{
> +    struct domain **pd;
> +
> +    spin_lock(&domlist_update_lock);
> +
> +    /* domain_list is maintained in domid order. */
> +    pd = &domain_list;

Make this the initializer of the variable, if ...

> +    for ( ; *pd != NULL; pd = &(*pd)->next_in_list )

... isn't to be the starting clause of the for()?

> +        if ( (*pd)->domain_id > d->domain_id )
> +            break;
> +
> +    d->next_in_list = *pd;
> +    d->next_in_hashbucket = domain_hash[DOMAIN_HASH(d->domain_id)];
> +    rcu_assign_pointer(*pd, d);
> +    rcu_assign_pointer(domain_hash[DOMAIN_HASH(d->domain_id)], d);

Maybe worth putting the hash in a local variable?

> +    spin_unlock(&domlist_update_lock);
> +}
> +
> +/*
> + * Remove a domain from the domlist/hash.  This means the domain can no 
> longer
> + * be looked up by domid, and therefore can no longer be the subject of
> + * *subsequent* hypercalls/etc.  In-progress hypercalls/etc can still operate
> + * on the domain.
> + */
> +static void domlist_remove(struct domain *d)
> +{
> +    struct domain **pd = &domain_list;
> +
> +    spin_lock(&domlist_update_lock);
> +
> +    pd = &domain_list;

pd already has an initializer.

With at least the pd related adjustments
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan