diff mbox series

page-alloc: accompany BUG() with printk()

Message ID 5CDBD157020000780022F1EA@prv1-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show
Series page-alloc: accompany BUG() with printk() | expand

Commit Message

Jan Beulich May 15, 2019, 8:44 a.m. UTC
Log information likely relevant for understanding why the BUG()s were
triggering.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Wei Liu May 15, 2019, 9:30 a.m. UTC | #1
On Wed, May 15, 2019 at 02:44:07AM -0600, Jan Beulich wrote:
> Log information likely relevant for understanding why the BUG()s were
> triggering.
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 

Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Roger Pau Monné May 15, 2019, 9:44 a.m. UTC | #2
On Wed, May 15, 2019 at 02:44:07AM -0600, Jan Beulich wrote:
> Log information likely relevant for understanding why the BUG()s were

Why not use panic instead of printk + BUG?

Thanks, Roger.
Andrew Cooper May 15, 2019, 9:59 a.m. UTC | #3
On 15/05/2019 10:44, Roger Pau Monné wrote:
> On Wed, May 15, 2019 at 02:44:07AM -0600, Jan Beulich wrote:
>> Log information likely relevant for understanding why the BUG()s were
> Why not use panic instead of printk + BUG?

Because the backtrace is useful a lot of the time.

~Andrew
Andrew Cooper May 15, 2019, 10:03 a.m. UTC | #4
On 15/05/2019 09:44, Jan Beulich wrote:
> Log information likely relevant for understanding why the BUG()s were
> triggering.
>
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citirx.com>
Roger Pau Monné May 15, 2019, 10:09 a.m. UTC | #5
On Wed, May 15, 2019 at 10:59:20AM +0100, Andrew Cooper wrote:
> On 15/05/2019 10:44, Roger Pau Monné wrote:
> > On Wed, May 15, 2019 at 02:44:07AM -0600, Jan Beulich wrote:
> >> Log information likely relevant for understanding why the BUG()s were
> > Why not use panic instead of printk + BUG?
> 
> Because the backtrace is useful a lot of the time.

Oh, panic has no backtrace, that's not very useful. Maybe we could add
a panic_trace version to avoid having to do a printk + BUG. In any
case:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.
Andrew Cooper May 15, 2019, 10:12 a.m. UTC | #6
On 15/05/2019 11:09, Roger Pau Monné wrote:
> On Wed, May 15, 2019 at 10:59:20AM +0100, Andrew Cooper wrote:
>> On 15/05/2019 10:44, Roger Pau Monné wrote:
>>> On Wed, May 15, 2019 at 02:44:07AM -0600, Jan Beulich wrote:
>>>> Log information likely relevant for understanding why the BUG()s were
>>> Why not use panic instead of printk + BUG?
>> Because the backtrace is useful a lot of the time.
> Oh, panic has no backtrace, that's not very useful. Maybe we could add
> a panic_trace version to avoid having to do a printk + BUG. In any
> case:

TBH, It would be better for BUG() to take a printk-like , but arranging
this to work is decidedly non-trivial, because it would have to be
arranged from #UD context with some way of passing va_args() behind the
scene.

~Andrew
diff mbox series

Patch

--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -984,7 +984,15 @@  static struct page_info *alloc_heap_page
     for ( i = 0; i < (1 << order); i++ )
     {
         /* Reference count must continuously be zero for free pages. */
-        BUG_ON((pg[i].count_info & ~PGC_need_scrub) != PGC_state_free);
+        if ( (pg[i].count_info & ~PGC_need_scrub) != PGC_state_free )
+        {
+            printk(XENLOG_ERR
+                   "pg[%u] MFN %"PRI_mfn" c=%#lx o=%u v=%#lx t=%#x\n",
+                   i, mfn_x(page_to_mfn(pg + i)),
+                   pg[i].count_info, pg[i].v.free.order,
+                   pg[i].u.free.val, pg[i].tlbflush_timestamp);
+            BUG();
+        }
 
         /* PGC_need_scrub can only be set if first_dirty is valid */
         ASSERT(first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub));
@@ -1393,6 +1401,11 @@  static void free_heap_pages(
             break;
 
         default:
+            printk(XENLOG_ERR
+                   "pg[%u] MFN %"PRI_mfn" c=%#lx o=%u v=%#lx t=%#x\n",
+                   i, mfn_x(page_to_mfn(pg + i)),
+                   pg[i].count_info, pg[i].v.free.order,
+                   pg[i].u.free.val, pg[i].tlbflush_timestamp);
             BUG();
         }
 
@@ -2336,7 +2349,15 @@  void free_domheap_pages(struct page_info
 
             for ( i = 0; i < (1 << order); i++ )
             {
-                BUG_ON((pg[i].u.inuse.type_info & PGT_count_mask) != 0);
+                if ( pg[i].u.inuse.type_info & PGT_count_mask )
+                {
+                    printk(XENLOG_ERR
+                           "pg[%u] MFN %"PRI_mfn" c=%#lx o=%u v=%#lx t=%#x\n",
+                           i, mfn_x(page_to_mfn(pg + i)),
+                           pg[i].count_info, pg[i].v.free.order,
+                           pg[i].u.free.val, pg[i].tlbflush_timestamp);
+                    BUG();
+                }
                 arch_free_heap_page(d, &pg[i]);
             }