Re: [Xen-devel] [PATCH 4/4] xen: modify lock profiling interface

[Jan Beulich <JBeulich@xxxxxxxx>]

On 07.08.2019 16:31, Juergen Gross wrote:
> @@ -463,31 +464,67 @@ int spinlock_profile_control(struct 
> xen_sysctl_lockprof_op *pc)
>       return rc;
>   }
>   
> -void _lock_profile_register_struct(
> -    int32_t type, struct lock_profile_qhead *qhead, int32_t idx, char *name)
> +static struct lock_profile_anc *find_prof_anc(const char *name)
>   {
> -    qhead->idx = idx;
> +    struct lock_profile_anc *anc;
> +
> +    for ( anc = lock_profile_ancs; anc; anc = anc->next )
> +        if ( !strcmp(anc->name, name) )
> +            return anc;
> +    return NULL;
> +}
> +
> +void _lock_profile_register_struct(struct lock_profile_qhead *qhead,
> +                                   int32_t idx, const char *name)
> +{
> +    struct lock_profile_anc *anc;
> +
>       spin_lock(&lock_profile_lock);
> -    qhead->head_q = lock_profile_ancs[type].head_q;
> -    lock_profile_ancs[type].head_q = qhead;
> -    lock_profile_ancs[type].name = name;
> +
> +    anc = find_prof_anc(name);
> +    if ( !anc )
> +    {
> +        anc = xzalloc(struct lock_profile_anc);
> +        anc->name = name;
> +        anc->next = lock_profile_ancs;
> +        lock_profile_ancs = anc;
> +    }

This is an imo fundamental weakness of the new model: You now
require a dynamic memory allocation (of which you don't even
check that it was successful). Right now the path above will
only be taken at boot time, but that's not stated anywhere as
a restriction (afaics), and hence doesn't need to remain this
way.

Furthermore "name" now serves two purposes when previously it
served just one. This is best noticeable with the function's
use in domain_create(): Previously that set up the class
"per-domain" with an initial instance "struct domain". This
did provide for someone later going and also registering
another per-domain structure (e.g. struct p2m_domain) as
another "per-domain" class instance. Then again maybe I'm not
correctly understanding the original intentions ...

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