Macros are defined to avoid type mismatch in format strings
but also to unify format amongst code.
In the meantime expands to 9 hexadecimal digits.
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
xen/common/grant_table.c | 6 +++---
xen/include/xen/mm-frame.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index ab36f45ded..775cd7e065 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1848,7 +1848,7 @@ gnttab_unpopulate_status_frames(struct domain *d, struct grant_table *gt)
if ( rc )
{
gprintk(XENLOG_ERR,
- "Could not remove status frame %u (GFN %#lx) from P2M\n",
+ "Could not remove status frame %u (GFN %"PRI_gfn") from P2M\n",
i, gfn_x(gfn));
domain_crash(d);
return rc;
@@ -1863,7 +1863,7 @@ gnttab_unpopulate_status_frames(struct domain *d, struct grant_table *gt)
if ( paging_mode_translate(d) )
{
gprintk(XENLOG_ERR,
- "Wrong page state %#lx of status frame %u (GFN %#lx)\n",
+ "Wrong page state %#lx of status frame %u (GFN %"PRI_gfn")\n",
pg->count_info, i, gfn_x(gfn));
domain_crash(d);
}
@@ -3981,7 +3981,7 @@ void grant_table_warn_active_grants(struct domain *d)
if ( nr_active <= WARN_GRANT_MAX )
printk(XENLOG_G_DEBUG "d%d has active grant %x ("
#ifndef NDEBUG
- "GFN %lx, "
+ "GFN %"PRI_gfn", "
#endif
"MFN: %#"PRI_mfn")\n",
d->domain_id, ref,
diff --git a/xen/include/xen/mm-frame.h b/xen/include/xen/mm-frame.h
index d973aec901..6f9fcc2a6a 100644
--- a/xen/include/xen/mm-frame.h
+++ b/xen/include/xen/mm-frame.h
@@ -5,7 +5,7 @@
#include <xen/typesafe.h>
TYPE_SAFE(unsigned long, mfn);
-#define PRI_mfn "05lx"
+#define PRI_mfn "09lx"
#define INVALID_MFN_RAW (~0UL)
#define INVALID_MFN _mfn(INVALID_MFN_RAW)
/*
@@ -41,7 +41,7 @@ static inline bool mfn_eq(mfn_t x, mfn_t y)
}
TYPE_SAFE(unsigned long, gfn);
-#define PRI_gfn "05lx"
+#define PRI_gfn "09lx"
#define INVALID_GFN_RAW (~0UL)
#define INVALID_GFN _gfn(INVALID_GFN_RAW)
/*
--
2.34.1On 09.09.2024 12:08, Frediano Ziglio wrote:
> Macros are defined to avoid type mismatch in format strings
> but also to unify format amongst code.
I'm certainly fine with this part.
> In the meantime expands to 9 hexadecimal digits.
What makes 9 special? What will the extra padding zeroes buy us?
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> ---
> xen/common/grant_table.c | 6 +++---
> xen/include/xen/mm-frame.h | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index ab36f45ded..775cd7e065 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1848,7 +1848,7 @@ gnttab_unpopulate_status_frames(struct domain *d, struct grant_table *gt)
> if ( rc )
> {
> gprintk(XENLOG_ERR,
> - "Could not remove status frame %u (GFN %#lx) from P2M\n",
> + "Could not remove status frame %u (GFN %"PRI_gfn") from P2M\n",
The lost # means the number won't identify itself as hex anymore. Rather
than ...
> @@ -3981,7 +3981,7 @@ void grant_table_warn_active_grants(struct domain *d)
> if ( nr_active <= WARN_GRANT_MAX )
> printk(XENLOG_G_DEBUG "d%d has active grant %x ("
> #ifndef NDEBUG
> - "GFN %lx, "
> + "GFN %"PRI_gfn", "
> #endif
> "MFN: %#"PRI_mfn")\n",
(note this for below)
> --- a/xen/include/xen/mm-frame.h
> +++ b/xen/include/xen/mm-frame.h
> @@ -5,7 +5,7 @@
> #include <xen/typesafe.h>
>
> TYPE_SAFE(unsigned long, mfn);
> -#define PRI_mfn "05lx"
> +#define PRI_mfn "09lx"
> #define INVALID_MFN_RAW (~0UL)
> #define INVALID_MFN _mfn(INVALID_MFN_RAW)
> /*
> @@ -41,7 +41,7 @@ static inline bool mfn_eq(mfn_t x, mfn_t y)
> }
>
> TYPE_SAFE(unsigned long, gfn);
> -#define PRI_gfn "05lx"
> +#define PRI_gfn "09lx"
... moving to 09 (twice) here, how about we move to #? Requiring, of course,
to drop already-questionable hashes like the one pointed out in the middle.
Jan
On Mon, Sep 9, 2024 at 11:45 AM Jan Beulich <jbeulich@suse.com> wrote:
> On 09.09.2024 12:08, Frediano Ziglio wrote:
> > Macros are defined to avoid type mismatch in format strings
> > but also to unify format amongst code.
>
> I'm certainly fine with this part.
>
> > In the meantime expands to 9 hexadecimal digits.
>
> What makes 9 special? What will the extra padding zeroes buy us?
>
>
I think either we want kind of fixed size or dynamic. 9 == (48 - 12) / 4,
although possibly you would prefer 13 == (64 - 12) / 4.
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> > ---
> > xen/common/grant_table.c | 6 +++---
> > xen/include/xen/mm-frame.h | 4 ++--
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> > index ab36f45ded..775cd7e065 100644
> > --- a/xen/common/grant_table.c
> > +++ b/xen/common/grant_table.c
> > @@ -1848,7 +1848,7 @@ gnttab_unpopulate_status_frames(struct domain *d,
> struct grant_table *gt)
> > if ( rc )
> > {
> > gprintk(XENLOG_ERR,
> > - "Could not remove status frame %u (GFN %#lx)
> from P2M\n",
> > + "Could not remove status frame %u (GFN
> %"PRI_gfn") from P2M\n",
>
> The lost # means the number won't identify itself as hex anymore. Rather
> than ...
>
> > @@ -3981,7 +3981,7 @@ void grant_table_warn_active_grants(struct domain
> *d)
> > if ( nr_active <= WARN_GRANT_MAX )
> > printk(XENLOG_G_DEBUG "d%d has active grant %x ("
> > #ifndef NDEBUG
> > - "GFN %lx, "
> > + "GFN %"PRI_gfn", "
> > #endif
> > "MFN: %#"PRI_mfn")\n",
>
> (note this for below)
>
> > --- a/xen/include/xen/mm-frame.h
> > +++ b/xen/include/xen/mm-frame.h
> > @@ -5,7 +5,7 @@
> > #include <xen/typesafe.h>
> >
> > TYPE_SAFE(unsigned long, mfn);
> > -#define PRI_mfn "05lx"
> > +#define PRI_mfn "09lx"
> > #define INVALID_MFN_RAW (~0UL)
> > #define INVALID_MFN _mfn(INVALID_MFN_RAW)
> > /*
> > @@ -41,7 +41,7 @@ static inline bool mfn_eq(mfn_t x, mfn_t y)
> > }
> >
> > TYPE_SAFE(unsigned long, gfn);
> > -#define PRI_gfn "05lx"
> > +#define PRI_gfn "09lx"
>
> ... moving to 09 (twice) here, how about we move to #? Requiring, of
> course,
> to drop already-questionable hashes like the one pointed out in the middle.
>
>
I suppose you are suggesting getting rid of the padding entirely and move
to prefix, like "%#lx".
> Jan
>
I can do it
Frediano
On 09.09.2024 14:53, Frediano Ziglio wrote:
> On Mon, Sep 9, 2024 at 11:45 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 09.09.2024 12:08, Frediano Ziglio wrote:
>>> Macros are defined to avoid type mismatch in format strings
>>> but also to unify format amongst code.
>>
>> I'm certainly fine with this part.
>>
>>> In the meantime expands to 9 hexadecimal digits.
>>
>> What makes 9 special? What will the extra padding zeroes buy us?
>>
>>
> I think either we want kind of fixed size or dynamic. 9 == (48 - 12) / 4,
> although possibly you would prefer 13 == (64 - 12) / 4.
64 is too much for x86; it would want to be 52 there. The way it is right
now this is (imo deliberately) not arch-specific, though.
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -1848,7 +1848,7 @@ gnttab_unpopulate_status_frames(struct domain *d,
>> struct grant_table *gt)
>>> if ( rc )
>>> {
>>> gprintk(XENLOG_ERR,
>>> - "Could not remove status frame %u (GFN %#lx)
>> from P2M\n",
>>> + "Could not remove status frame %u (GFN
>> %"PRI_gfn") from P2M\n",
>>
>> The lost # means the number won't identify itself as hex anymore. Rather
>> than ...
>>
>>> @@ -3981,7 +3981,7 @@ void grant_table_warn_active_grants(struct domain
>> *d)
>>> if ( nr_active <= WARN_GRANT_MAX )
>>> printk(XENLOG_G_DEBUG "d%d has active grant %x ("
>>> #ifndef NDEBUG
>>> - "GFN %lx, "
>>> + "GFN %"PRI_gfn", "
>>> #endif
>>> "MFN: %#"PRI_mfn")\n",
>>
>> (note this for below)
>>
>>> --- a/xen/include/xen/mm-frame.h
>>> +++ b/xen/include/xen/mm-frame.h
>>> @@ -5,7 +5,7 @@
>>> #include <xen/typesafe.h>
>>>
>>> TYPE_SAFE(unsigned long, mfn);
>>> -#define PRI_mfn "05lx"
>>> +#define PRI_mfn "09lx"
>>> #define INVALID_MFN_RAW (~0UL)
>>> #define INVALID_MFN _mfn(INVALID_MFN_RAW)
>>> /*
>>> @@ -41,7 +41,7 @@ static inline bool mfn_eq(mfn_t x, mfn_t y)
>>> }
>>>
>>> TYPE_SAFE(unsigned long, gfn);
>>> -#define PRI_gfn "05lx"
>>> +#define PRI_gfn "09lx"
>>
>> ... moving to 09 (twice) here, how about we move to #? Requiring, of
>> course,
>> to drop already-questionable hashes like the one pointed out in the middle.
>>
> I suppose you are suggesting getting rid of the padding entirely and move
> to prefix, like "%#lx".
Yes, i.e.
#define PRI_mfn "#lx"
Then again I don't really know why "05lx" was chosen originally.
Jan
On Mon, Sep 9, 2024 at 1:58 PM Jan Beulich <jbeulich@suse.com> wrote:
> On 09.09.2024 14:53, Frediano Ziglio wrote:
> > On Mon, Sep 9, 2024 at 11:45 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 09.09.2024 12:08, Frediano Ziglio wrote:
> >>> Macros are defined to avoid type mismatch in format strings
> >>> but also to unify format amongst code.
> >>
> >> I'm certainly fine with this part.
> >>
> >>> In the meantime expands to 9 hexadecimal digits.
> >>
> >> What makes 9 special? What will the extra padding zeroes buy us?
> >>
> >>
> > I think either we want kind of fixed size or dynamic. 9 == (48 - 12) / 4,
> > although possibly you would prefer 13 == (64 - 12) / 4.
>
> 64 is too much for x86; it would want to be 52 there. The way it is right
> now this is (imo deliberately) not arch-specific, though.
>
>
Yes, but still given the canonic form of x64 you would need to use 13
digits to have all the same size.
> >>> --- a/xen/common/grant_table.c
> >>> +++ b/xen/common/grant_table.c
> >>> @@ -1848,7 +1848,7 @@ gnttab_unpopulate_status_frames(struct domain *d,
> >> struct grant_table *gt)
> >>> if ( rc )
> >>> {
> >>> gprintk(XENLOG_ERR,
> >>> - "Could not remove status frame %u (GFN %#lx)
> >> from P2M\n",
> >>> + "Could not remove status frame %u (GFN
> >> %"PRI_gfn") from P2M\n",
> >>
> >> The lost # means the number won't identify itself as hex anymore. Rather
> >> than ...
> >>
> >>> @@ -3981,7 +3981,7 @@ void grant_table_warn_active_grants(struct domain
> >> *d)
> >>> if ( nr_active <= WARN_GRANT_MAX )
> >>> printk(XENLOG_G_DEBUG "d%d has active grant %x ("
> >>> #ifndef NDEBUG
> >>> - "GFN %lx, "
> >>> + "GFN %"PRI_gfn", "
> >>> #endif
> >>> "MFN: %#"PRI_mfn")\n",
> >>
> >> (note this for below)
> >>
> >>> --- a/xen/include/xen/mm-frame.h
> >>> +++ b/xen/include/xen/mm-frame.h
> >>> @@ -5,7 +5,7 @@
> >>> #include <xen/typesafe.h>
> >>>
> >>> TYPE_SAFE(unsigned long, mfn);
> >>> -#define PRI_mfn "05lx"
> >>> +#define PRI_mfn "09lx"
> >>> #define INVALID_MFN_RAW (~0UL)
> >>> #define INVALID_MFN _mfn(INVALID_MFN_RAW)
> >>> /*
> >>> @@ -41,7 +41,7 @@ static inline bool mfn_eq(mfn_t x, mfn_t y)
> >>> }
> >>>
> >>> TYPE_SAFE(unsigned long, gfn);
> >>> -#define PRI_gfn "05lx"
> >>> +#define PRI_gfn "09lx"
> >>
> >> ... moving to 09 (twice) here, how about we move to #? Requiring, of
> >> course,
> >> to drop already-questionable hashes like the one pointed out in the
> middle.
> >>
> > I suppose you are suggesting getting rid of the padding entirely and move
> > to prefix, like "%#lx".
>
> Yes, i.e.
>
> #define PRI_mfn "#lx"
>
> Surely more portable amongst different platforms.
> Then again I don't really know why "05lx" was chosen originally.
>
>
I assume x86 without PAE, 32 bit, so 5 == (32 - 12) / 4.
> Jan
>
Sent updated one
Frediano
© 2016 - 2024 Red Hat, Inc.