Re: [PATCH v5 01/14] rangeset: add RANGESETF_no_print flag

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

Hi, Volodymyr!

On 15.12.21 05:20, Volodymyr Babchuk wrote:
> Hi Oleksandr,
>
> Sorry for jumping in amid v5, but...
>
> Oleksandr Andrushchenko <andr2000@xxxxxxxxx> writes:
>
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>
>> There are range sets which should not be printed, so introduce a flag
>> which allows marking those as such. Implement relevant logic to skip
>> such entries while printing.
>>
>> While at it also simplify the definition of the flags by directly
>> defining those without helpers.
>>
>> Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>
>> ---
>> Since v1:
>> - update BUG_ON with new flag
>> - simplify the definition of the flags
>> ---
>>   xen/common/rangeset.c      | 5 ++++-
>>   xen/include/xen/rangeset.h | 7 ++++---
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
>> index 885b6b15c229..ea27d651723b 100644
>> --- a/xen/common/rangeset.c
>> +++ b/xen/common/rangeset.c
>> @@ -433,7 +433,7 @@ struct rangeset *rangeset_new(
>>       INIT_LIST_HEAD(&r->range_list);
>>       r->nr_ranges = -1;
>>   
>> -    BUG_ON(flags & ~RANGESETF_prettyprint_hex);
>> +    BUG_ON(flags & ~(RANGESETF_prettyprint_hex | RANGESETF_no_print));
>>       r->flags = flags;
>>   
>>       safe_strcpy(r->name, name ?: "(no name)");
>> @@ -575,6 +575,9 @@ void rangeset_domain_printk(
>>   
>>       list_for_each_entry ( r, &d->rangesets, rangeset_list )
>>       {
>> +        if ( r->flags & RANGESETF_no_print )
>> +            continue;
>> +
>>           printk("    ");
>>           rangeset_printk(r);
>>           printk("\n");
>> diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
>> index 135f33f6066f..045fcafa8368 100644
>> --- a/xen/include/xen/rangeset.h
>> +++ b/xen/include/xen/rangeset.h
>> @@ -48,9 +48,10 @@ void rangeset_limit(
>>       struct rangeset *r, unsigned int limit);
>>   
>>   /* Flags for passing to rangeset_new(). */
>> - /* Pretty-print range limits in hexadecimal. */
>> -#define _RANGESETF_prettyprint_hex 0
>> -#define RANGESETF_prettyprint_hex  (1U << _RANGESETF_prettyprint_hex)
>> +/* Pretty-print range limits in hexadecimal. */
>> +#define RANGESETF_prettyprint_hex   (1U << 0)
> If you already touching all the flags, why not to use BIT()?
It was discussed previously [1] and we decided not to use the BIT macro

Thank you,
Oleksandr
>
>> +/* Do not print entries marked with this flag. */
>> +#define RANGESETF_no_print          (1U << 1)
>>   
>>   bool_t __must_check rangeset_is_empty(
>>       const struct rangeset *r);
>
[1] 
https://patchwork.kernel.org/project/xen-devel/patch/20211122092825.2502306-1-andr2000@xxxxxxxxx/