[GIT] Grant table fix

[GIT] Grant table fix

[Ian Campbell]

The following changes since commit ab77527f9a63a5c657c6d6a50e70a66adceaa3a0:
  Jeremy Fitzhardinge (1):
        xen/blktap2: make compile

are available in the git repository at:

  git://xenbits.xensource.com/people/ianc/linux-2.6.git for-jeremy/grant-table

Ian Campbell (1):
      gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.

 drivers/xen/grant-table.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

[PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.

[Ian Campbell]

Otherwise we trip over the check for PAGE_FLAGS_CHECK_AT_FREE in
free_pages_check() when finally freeing the page, leading to
backtraces such as:
    Bad page state in process 'tcpdump'
    page:c15b8ae0 flags:0x40000800 mapping:00000000 mapcount:0 count:0
    Trying to fix it up, but a reboot is needed
    Backtrace:
    Pid: 5731, comm: tcpdump Tainted: G          2.6.27.42-0.1.1.xs5.5.900.751.1073xen #1
     [<c015daeb>] bad_page+0x6b/0xa0
     [<c015e389>] free_hot_cold_page+0x239/0x250
     [<c015e3ea>] free_hot_page+0xa/0x10
     [<c0162255>] put_page+0x35/0xc0
     [<c026e002>] gnttab_page_free+0x22/0x30
     [<c015e325>] free_hot_cold_page+0x1d5/0x250
     [<c015e3ea>] free_hot_page+0xa/0x10
     [<c0162255>] put_page+0x35/0xc0
     [<c02cbe4a>] skb_put_page+0xa/0x10
     [<c02cc0b7>] skb_release_data+0x77/0x90
     [<c02cc78b>] skb_release_all+0x6b/0xa0
     [<c02cbf3b>] __kfree_skb+0xb/0x80
     [<c02cbfce>] kfree_skb+0x1e/0x40
     [<c02ce9bd>] skb_free_datagram+0xd/0x40
     [<c03360a6>] packet_recvmsg+0x186/0x1c0
     [<c015d8fb>] ? __rmqueue+0x1b/0x1a0
     [<c02c6222>] sock_recvmsg+0x102/0x130
     [<c013de50>] ? autoremove_wake_function+0x0/0x50
     [<c01691e7>] ? __do_fault+0x2e7/0x5f0
     [<c02c5af0>] ? sockfd_lookup_light+0x30/0x60
     [<c02c707d>] sys_recvfrom+0x7d/0xe0
     [<c0180dc9>] ? __kmalloc+0x139/0x190
     [<c02074bc>] ? copy_from_user+0x3c/0x70
     [<c03489d4>] ? _spin_lock_bh+0x14/0x70
     [<c03484c3>] ? _spin_unlock_bh+0x23/0x30
     [<c02c83df>] ? release_sock+0x9f/0xc0
     [<c02c7116>] sys_recv+0x36/0x40
     [<c02c759f>] sys_socketcall+0x15f/0x290
     [<c01053ce>] syscall_call+0x7/0xb
     [<c0340000>] ? pci_scan_bus_on_node+0x10/0x80
     =======================

gnttab_copy_grant_page is (currently) only ever used on pages which
were allocated by alloc_empty_pages_and_pagevec() and hence have the
PG_reserved set. Also free_empty_pages_and_pagevec() can
BUG_ON(!PageReserved(page)).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/grant-table.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 17efd09..7079787 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -558,9 +558,12 @@ int gnttab_copy_grant_page(grant_ref_t ref, struct page **pagep)
 	new_page->mapping = page->mapping;
 	new_page->index = page->index;
 	set_bit(PG_foreign, &new_page->flags);
+	if (PageReserved(page))
+		set_bit(PG_reserved, &new_page->flags);
 	*pagep = new_page;
 
 	SetPageForeign(page, gnttab_page_free);
+	ClearPageReserved(page);
 	page->mapping = NULL;
 
 out:
-- 
1.5.6.5

Re: [PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.

[Jan Beulich]

>>> Ian Campbell <ian.campbell@citrix.com> 23.02.10 17:40 >>>
>--- a/drivers/xen/grant-table.c
>+++ b/drivers/xen/grant-table.c
>@@ -558,9 +558,12 @@ int gnttab_copy_grant_page(grant_ref_t ref, struct page **pagep)
> 	new_page->mapping = page->mapping;
> 	new_page->index = page->index;
> 	set_bit(PG_foreign, &new_page->flags);
>+	if (PageReserved(page))
>+		set_bit(PG_reserved, &new_page->flags);

Why not SetPageReserved()?

Jan

Re: [PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.

[Ian Campbell]

On Tue, 2010-02-23 at 17:04 +0000, Jan Beulich wrote:
> >>> Ian Campbell <ian.campbell@citrix.com> 23.02.10 17:40 >>>
> >--- a/drivers/xen/grant-table.c
> >+++ b/drivers/xen/grant-table.c
> >@@ -558,9 +558,12 @@ int gnttab_copy_grant_page(grant_ref_t ref, struct page **pagep)
> > 	new_page->mapping = page->mapping;
> > 	new_page->index = page->index;
> > 	set_bit(PG_foreign, &new_page->flags);
> >+	if (PageReserved(page))
> >+		set_bit(PG_reserved, &new_page->flags);
> 
> Why not SetPageReserved()?

I was just following the pattern above with PG_foreign. I guess that is
subtly different since either mapping or index (I forget which) would
need to be the second argument to SetPageForeign (probably an accessor
is required for that dtor field).

This function is grubbing around at a low level with many of the struct
page fields -- I guess doing it this way makes it a little more obvious
that something subtle is going on but I'm not fussed about changing it.

I'll follow up with something which fixes this up for both reserved and
foreign.

Ian.

[PATCH] grant-table: use page flag interfaces when copying a grant page

[Ian Campbell]

Use SetPage{Foreign,Reserved} instead of bit-bashing directly. Add an
accessor for the foreign page destructor.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/grant-table.c  |    5 ++---
 include/linux/page-flags.h |    4 +++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 7079787..76fe621 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -556,10 +556,9 @@ int gnttab_copy_grant_page(grant_ref_t ref, struct page **pagep)
 	}
 
 	new_page->mapping = page->mapping;
-	new_page->index = page->index;
-	set_bit(PG_foreign, &new_page->flags);
+	SetPageForeign(new_page, _PageForeignDestructor(page));
 	if (PageReserved(page))
-		set_bit(PG_reserved, &new_page->flags);
+		SetPageReserved(new_page);
 	*pagep = new_page;
 
 	SetPageForeign(page, gnttab_page_free);
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 86325f9..b03950e 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -287,8 +287,10 @@ CLEARPAGEFLAG(Foreign, foreign)
 	BUG_ON((dtor) == (void (*)(struct page *, unsigned int))0);	\
 	(_page)->index = (long)(dtor);					\
 } while (0)
+#define _PageForeignDestructor(_page) \
+	((void (*)(struct page *, unsigned int))(_page)->index)
 #define PageForeignDestructor(_page, order)	\
-	((void (*)(struct page *, unsigned int))(_page)->index)(_page, order)
+	_PageForeignDestructor(_page)(_page, order)
 #else
 PAGEFLAG_FALSE(Foreign)
 #endif
-- 
1.5.6.5

Re: [PATCH] grant-table: use page flag interfaces when copying a grant page

[Ian Campbell]

Updated pull request with this additional changeset is:

The following changes since commit ab77527f9a63a5c657c6d6a50e70a66adceaa3a0:
  Jeremy Fitzhardinge (1):
        xen/blktap2: make compile

are available in the git repository at:

  git://xenbits.xensource.com/people/ianc/linux-2.6.git for-jeremy/grant-table

Ian Campbell (2):
      gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.
      grant-table: use page flag interfaces when copying a grant page

 drivers/xen/grant-table.c  |    6 ++++--
 include/linux/page-flags.h |    4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

Re: [GIT] Grant table fix

[Jeremy Fitzhardinge]

On 02/23/2010 08:39 AM, Ian Campbell wrote:
> The following changes since commit ab77527f9a63a5c657c6d6a50e70a66adceaa3a0:
>    Jeremy Fitzhardinge (1):
>          xen/blktap2: make compile
>
> are available in the git repository at:
>
>    git://xenbits.xensource.com/people/ianc/linux-2.6.git for-jeremy/grant-table
>
> Ian Campbell (1):
>        gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.
>
>   drivers/xen/grant-table.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
>
>    

Is this needed for xen/master too?

     J

Re: [GIT] Grant table fix

[Ian Campbell]

On Tue, 2010-02-23 at 18:02 +0000, Jeremy Fitzhardinge wrote:
> On 02/23/2010 08:39 AM, Ian Campbell wrote:
> > The following changes since commit ab77527f9a63a5c657c6d6a50e70a66adceaa3a0:
> >    Jeremy Fitzhardinge (1):
> >          xen/blktap2: make compile
> >
> > are available in the git repository at:
> >
> >    git://xenbits.xensource.com/people/ianc/linux-2.6.git for-jeremy/grant-table
> >
> > Ian Campbell (1):
> >        gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.
> >
> >   drivers/xen/grant-table.c |    3 +++
> >   1 files changed, 3 insertions(+), 0 deletions(-)
> >
> >    
> 
> Is this needed for xen/master too?

Netback is the only caller of this function so I think it's OK not to
for now. (unless netback is in xen/master since I last looked)

Ian.