Re: [PATCH 2/4] rangeset: add rangeset_reset helper function

[Julien Grall <julien@xxxxxxx>]

Hi,

On 01/02/2022 16:25, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>
> This helper destroys all the ranges of the rangeset given.
> Please note, that it uses rangeset_remove_range which returns an error
> code on failure. This error code can be ignored as while destroying all
> the ranges no memory allocation is expected, so in this case it must not
> fail.
>
> To make sure this remains valid use BUG_ON if that changes in the future.
>
> Suggested-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> ---
>   xen/common/rangeset.c      | 6 ++++++
>   xen/include/xen/rangeset.h | 3 +++
>   2 files changed, 9 insertions(+)
>
> diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
> index ea27d651723b..9ca2b06cff22 100644
> --- a/xen/common/rangeset.c
> +++ b/xen/common/rangeset.c
> @@ -525,6 +525,12 @@ void rangeset_swap(struct rangeset *a, struct rangeset *b)
>       write_unlock(&b->lock);
>   }
>   
> +void rangeset_reset(struct rangeset *r)
> +{
> +    /* This doesn't allocate anything and must not fail. */
> +    BUG_ON(rangeset_remove_range(r, 0, ~0ULL));
I vaguely recall some discussion in the past (not related to this 
series) that we wanted to avoid calling function have side-effect in a 
BUG_ON(). So if we decide to remove at compile-time BUG_ON(), there 
would be no issue.
But then I am not sure about the use of BUG_ON(). Can you outline why 
crashing the hypervisor is better than continuing (e.g. use WARN_ON() or 
ASSERT())?
Cheers,

--
Julien Grall