diff mbox series

[Xen-devel,v3,11/14] xen/x86: p2m: Remove duplicate error message in p2m_pt_audit_p2m()

Message ID 20190603160350.29806-12-julien.grall@arm.com
State New
Headers show
Series xen/arm: Properly disable M2P on Arm | expand

Commit Message

Julien Grall June 3, 2019, 4:03 p.m. UTC
p2m_pt_audit_p2m() has one place where the same message may be printed
twice via printk and P2M_PRINTK.

Remove the one printed using printk to stay consistent with the rest of
the code.

Take the opportunity to reflow the format of P2M_PRINTK.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v3:
        - Patch added
---
 xen/arch/x86/mm/p2m-pt.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Jan Beulich June 5, 2019, 10:43 a.m. UTC | #1
>>> On 03.06.19 at 18:03, <julien.grall@arm.com> wrote:
> p2m_pt_audit_p2m() has one place where the same message may be printed
> twice via printk and P2M_PRINTK.
> 
> Remove the one printed using printk to stay consistent with the rest of
> the code.
> 
> Take the opportunity to reflow the format of P2M_PRINTK.

Hmm, yes, but ...

> --- a/xen/arch/x86/mm/p2m-pt.c
> +++ b/xen/arch/x86/mm/p2m-pt.c
> @@ -1041,9 +1041,8 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
>                          if ( m2pfn != (gfn + i2) )
>                          {
>                              pmbad++;
> -                            P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx"
> -                                       " -> gfn %#lx\n", gfn+i2, mfn+i2,
> -                                       m2pfn);
> +                            P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx -> gfn %#lx\n",
> +                                       gfn + i2, mfn + i2, m2pfn);

... you re-flow an unrelated (but similar) one while ...

> @@ -1108,8 +1107,6 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
>                               !p2m_is_shared(type) )
>                          {
>                              pmbad++;
> -                            printk("mismatch: gfn %#lx -> mfn %#lx"
> -                                   " -> gfn %#lx\n", gfn, mfn, m2pfn);
>                              P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx"
>                                         " -> gfn %#lx\n", gfn, mfn, m2pfn);

... you leave alone this one. I don't mind touching the other
one, but this one surely wants touching then as well. And if
you touch that other one, then I think for consistency you
should also touch the 3rd one (between the two).

Jan
Julien Grall June 5, 2019, 10:44 a.m. UTC | #2
Hi Jan,

On 05/06/2019 11:43, Jan Beulich wrote:
>>>> On 03.06.19 at 18:03, <julien.grall@arm.com> wrote:
>> p2m_pt_audit_p2m() has one place where the same message may be printed
>> twice via printk and P2M_PRINTK.
>>
>> Remove the one printed using printk to stay consistent with the rest of
>> the code.
>>
>> Take the opportunity to reflow the format of P2M_PRINTK.
> 
> Hmm, yes, but ...

This is a mistake when I wrote the patch/rebase.

> 
>> --- a/xen/arch/x86/mm/p2m-pt.c
>> +++ b/xen/arch/x86/mm/p2m-pt.c
>> @@ -1041,9 +1041,8 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
>>                           if ( m2pfn != (gfn + i2) )
>>                           {
>>                               pmbad++;
>> -                            P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx"
>> -                                       " -> gfn %#lx\n", gfn+i2, mfn+i2,
>> -                                       m2pfn);
>> +                            P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx -> gfn %#lx\n",
>> +                                       gfn + i2, mfn + i2, m2pfn);
> 
> ... you re-flow an unrelated (but similar) one while ...
> 
>> @@ -1108,8 +1107,6 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
>>                                !p2m_is_shared(type) )
>>                           {
>>                               pmbad++;
>> -                            printk("mismatch: gfn %#lx -> mfn %#lx"
>> -                                   " -> gfn %#lx\n", gfn, mfn, m2pfn);
>>                               P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx"
>>                                          " -> gfn %#lx\n", gfn, mfn, m2pfn);
> 
> ... you leave alone this one. I don't mind touching the other
> one, but this one surely wants touching then as well. And if
> you touch that other one, then I think for consistency you
> should also touch the 3rd one (between the two).

I will only re-flow this message.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index cafc9f299b..84ddc1834b 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -1041,9 +1041,8 @@  long p2m_pt_audit_p2m(struct p2m_domain *p2m)
                         if ( m2pfn != (gfn + i2) )
                         {
                             pmbad++;
-                            P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx"
-                                       " -> gfn %#lx\n", gfn+i2, mfn+i2,
-                                       m2pfn);
+                            P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx -> gfn %#lx\n",
+                                       gfn + i2, mfn + i2, m2pfn);
                             BUG();
                         }
                         gfn += 1 << (L3_PAGETABLE_SHIFT - PAGE_SHIFT);
@@ -1108,8 +1107,6 @@  long p2m_pt_audit_p2m(struct p2m_domain *p2m)
                              !p2m_is_shared(type) )
                         {
                             pmbad++;
-                            printk("mismatch: gfn %#lx -> mfn %#lx"
-                                   " -> gfn %#lx\n", gfn, mfn, m2pfn);
                             P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx"
                                        " -> gfn %#lx\n", gfn, mfn, m2pfn);
                             BUG();