Re: [PATCH 1/4] xen/cache-col: Remove bogus cast in domain_llc_coloring_free()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.07.2025 18:23, Andrew Cooper wrote:
> --- a/xen/common/llc-coloring.c
> +++ b/xen/common/llc-coloring.c
> @@ -309,11 +309,8 @@ int domain_set_llc_colors(struct domain *d,
>  
>  void domain_llc_coloring_free(struct domain *d)
>  {
> -    if ( !llc_coloring_enabled || d->llc_colors == default_colors )
> -        return;
> -
> -    /* free pointer-to-const using __va(__pa()) */
> -    xfree(__va(__pa(d->llc_colors)));
> +    if ( d->llc_colors != default_colors )
> +        xfree(d->llc_colors);
>  }
>  
>  int __init domain_set_llc_colors_from_str(struct domain *d, const char *str)
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index fe53d4fab7ba..df23411869e6 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -649,7 +649,7 @@ struct domain
>  
>  #ifdef CONFIG_LLC_COLORING
>      unsigned int num_llc_colors;
> -    const unsigned int *llc_colors;
> +    unsigned int *llc_colors;
>  #endif
>  
>      /* Console settings. */

Ah yes, I see. Yet no, I don't agree. The only sane course of action
to avoid odd transformations like the above (without using casts to
cast away const-ness) is to finally make xfree() et al take pointers
to const void. Arguments towards why this makes sense were given
before; I don't think they need repeating. Dropping the const here is
rather undesirable: Once set, the colors shouldn't be altered anymore.
Pointers like this hence want to be pointer-to-const, to make
accidental modification less likely. Which in turn calls for the
mentioned adjustment to xfree(). Which you keep objecting to for
reasons I sadly cannot follow.

Jan