| Message ID | 20211125110251.2877218-2-andr2000@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PCI devices passthrough on Arm, part 3 | expand |
On 25.11.2021 12:02, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > 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@suse.com> > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> albeit with a remark: > --- 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. */ I would guess this comment was intentionally indented by a blank, to visually separate it from the comment covering all flags. I'd prefer if that was kept and if the new comment you add followed suit. Jan
On 25.11.21 13:06, Jan Beulich wrote: > On 25.11.2021 12:02, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> 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@suse.com> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > albeit with a remark: > >> --- 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. */ > I would guess this comment was intentionally indented by a blank, > to visually separate it from the comment covering all flags. I'd > prefer if that was kept and if the new comment you add followed > suit. Ah, ok, so I will add a space for the new flag's comment as well then > Jan > Thank you, Oleksandr
Hi Oleksandr, Sorry for jumping in amid v5, but... Oleksandr Andrushchenko <andr2000@gmail.com> writes: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > 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@suse.com> > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > --- > 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()? > +/* 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);
Hi, Volodymyr! On 15.12.21 05:20, Volodymyr Babchuk wrote: > Hi Oleksandr, > > Sorry for jumping in amid v5, but... > > Oleksandr Andrushchenko <andr2000@gmail.com> writes: > >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> 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@suse.com> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> --- >> 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@gmail.com/
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) +/* 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);