Re: [PATCH v2 2/6] xen/sched: create public function for cpupools creation

[Juergen Gross <jgross@xxxxxxxx>]

On 10.03.22 18:10, Luca Fancellu wrote:
> Create new public function to create cpupools, can take as parameter
> the scheduler id or a negative value that means the default Xen
> scheduler will be used.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
> Changes in v2:
> - cpupool_create_pool doesn't check anymore for pool id uniqueness
>    before calling cpupool_create. Modified commit message accordingly
> ---
>   xen/common/sched/cpupool.c | 15 +++++++++++++++
>   xen/include/xen/sched.h    | 16 ++++++++++++++++
>   2 files changed, 31 insertions(+)
>
> diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
> index a6da4970506a..89a891af7076 100644
> --- a/xen/common/sched/cpupool.c
> +++ b/xen/common/sched/cpupool.c
> @@ -1219,6 +1219,21 @@ static void cpupool_hypfs_init(void)
>   
>   #endif /* CONFIG_HYPFS */
>   
> +struct cpupool *__init cpupool_create_pool(unsigned int pool_id, int sched_id)
> +{
> +    struct cpupool *pool;
> +
> +    if ( sched_id < 0 )
> +        sched_id = scheduler_get_default()->sched_id;
> +
> +    pool = cpupool_create(pool_id, sched_id);
> +
> +    BUG_ON(IS_ERR(pool));
> +    cpupool_put(pool);
> +
> +    return pool;
> +}
> +
>   static int __init cf_check cpupool_init(void)
>   {
>       unsigned int cpu;
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 10ea969c7af9..47fc856e0fe0 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -1145,6 +1145,22 @@ int cpupool_move_domain(struct domain *d, struct cpupool 
> *c);
>   int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op);
>   unsigned int cpupool_get_id(const struct domain *d);
>   const cpumask_t *cpupool_valid_cpus(const struct cpupool *pool);
> +
> +/*
> + * cpupool_create_pool - Creates a cpupool
> + * @pool_id: id of the pool to be created
> + * @sched_id: id of the scheduler to be used for the pool
> + *
> + * Creates a cpupool with pool_id id.
> + * The sched_id parameter identifies the scheduler to be used, if it is
> + * negative, the default scheduler of Xen will be used.
> + *
> + * returns:
> + *     pointer to the struct cpupool just created, on success
> + *     NULL, on cpupool creation error
Either add a "." in the previous line, or rephrase (e.g.:
"pointer to the struct cpupool just created, or NULL in case of error"

I happened to read it as "pointer to the struct cpupool just created,
on success NULL, on cpupool creation error" first, which was weird.

With that fixed:

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature