| Message ID | 20190927154628.8480-1-david@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c5ad81eb029570c5ca5859539b0679f07a776d25 |
| Headers | show |
| Series | [v1] xen/balloon: Set pages PageOffline() in balloon_add_region() | expand |
On 9/27/19 11:46 AM, David Hildenbrand wrote: > We are missing a __SetPageOffline(), which is why we can get > !PageOffline() pages onto the balloon list, where > alloc_xenballooned_pages() will complain: > > page:ffffea0003e7ffc0 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 > flags: 0xffffe00001000(reserved) > raw: 000ffffe00001000 dead000000000100 dead000000000200 0000000000000000 > raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000 > page dumped because: VM_BUG_ON_PAGE(!PageOffline(page)) > ------------[ cut here ]------------ > kernel BUG at include/linux/page-flags.h:744! > invalid opcode: 0000 [#1] SMP NOPTI > > Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > Fixes: 77c4adf6a6df ("xen/balloon: mark inflated pages PG_offline") > Cc: stable@vger.kernel.org # v5.1+ > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > Cc: Juergen Gross <jgross@suse.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > drivers/xen/balloon.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index 05b1f7e948ef..29f6256363db 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -687,6 +687,7 @@ static void __init balloon_add_region(unsigned long start_pfn, > /* totalram_pages and totalhigh_pages do not > include the boot-time balloon extension, so > don't subtract from it. */ > + __SetPageOffline(page); > __balloon_append(page); Given that when a page is appended to balloon list need to be marked offline and, conversely, when a page is retrieved the bit should be cleared I wonder whether it's better to move Set/ClearPageOffline to balloon_append/retrieve(). -boris
On 27.09.19 20:08, Boris Ostrovsky wrote: > On 9/27/19 11:46 AM, David Hildenbrand wrote: >> We are missing a __SetPageOffline(), which is why we can get >> !PageOffline() pages onto the balloon list, where >> alloc_xenballooned_pages() will complain: >> >> page:ffffea0003e7ffc0 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 >> flags: 0xffffe00001000(reserved) >> raw: 000ffffe00001000 dead000000000100 dead000000000200 0000000000000000 >> raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000 >> page dumped because: VM_BUG_ON_PAGE(!PageOffline(page)) >> ------------[ cut here ]------------ >> kernel BUG at include/linux/page-flags.h:744! >> invalid opcode: 0000 [#1] SMP NOPTI >> >> Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >> Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >> Fixes: 77c4adf6a6df ("xen/balloon: mark inflated pages PG_offline") >> Cc: stable@vger.kernel.org # v5.1+ >> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> Cc: Juergen Gross <jgross@suse.com> >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> drivers/xen/balloon.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c >> index 05b1f7e948ef..29f6256363db 100644 >> --- a/drivers/xen/balloon.c >> +++ b/drivers/xen/balloon.c >> @@ -687,6 +687,7 @@ static void __init balloon_add_region(unsigned long start_pfn, >> /* totalram_pages and totalhigh_pages do not >> include the boot-time balloon extension, so >> don't subtract from it. */ >> + __SetPageOffline(page); >> __balloon_append(page); > > > Given that when a page is appended to balloon list need to be marked > offline and, conversely, when a page is retrieved the bit should be > cleared I wonder whether it's better to move Set/ClearPageOffline to > balloon_append/retrieve(). Yes, we can do that as an addon patch (and also get rid of either __balloon_append() or balloon_append() if I remember correctly).
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c index 05b1f7e948ef..29f6256363db 100644 --- a/drivers/xen/balloon.c +++ b/drivers/xen/balloon.c @@ -687,6 +687,7 @@ static void __init balloon_add_region(unsigned long start_pfn, /* totalram_pages and totalhigh_pages do not include the boot-time balloon extension, so don't subtract from it. */ + __SetPageOffline(page); __balloon_append(page); }