All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT] Grant table fix
@ 2010-02-23 16:39 Ian Campbell
  2010-02-23 16:40 ` [PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page Ian Campbell
  2010-02-23 18:02 ` [GIT] Grant table fix Jeremy Fitzhardinge
  0 siblings, 2 replies; 8+ messages in thread
From: Ian Campbell @ 2010-02-23 16:39 UTC (permalink / raw)
  To: xen-devel; +Cc: Jeremy Fitzhardinge

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(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.
  2010-02-23 16:39 [GIT] Grant table fix Ian Campbell
@ 2010-02-23 16:40 ` Ian Campbell
  2010-02-23 17:04   ` Jan Beulich
  2010-02-23 18:02 ` [GIT] Grant table fix Jeremy Fitzhardinge
  1 sibling, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2010-02-23 16:40 UTC (permalink / raw)
  To: xen-devel; +Cc: Jeremy Fitzhardinge, 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page.
  2010-02-23 16:40 ` [PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page Ian Campbell
@ 2010-02-23 17:04   ` Jan Beulich
  2010-02-23 17:08     ` Ian Campbell
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2010-02-23 17:04 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: Jeremy Fitzhardinge

>>> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnttab: propagate Reserved flag from  old to new page in gnttab_copy_grant_page.
  2010-02-23 17:04   ` Jan Beulich
@ 2010-02-23 17:08     ` Ian Campbell
  2010-02-23 17:24       ` [PATCH] grant-table: use page flag interfaces when copying a grant page Ian Campbell
  0 siblings, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2010-02-23 17:08 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Fitzhardinge, xen-devel, Jeremy

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] grant-table: use page flag interfaces when copying a grant page
  2010-02-23 17:08     ` Ian Campbell
@ 2010-02-23 17:24       ` Ian Campbell
  2010-02-23 17:25         ` Ian Campbell
  0 siblings, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2010-02-23 17:24 UTC (permalink / raw)
  To: xen-devel; +Cc: Jeremy Fitzhardinge, Ian Campbell, Jan Beulich

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] grant-table: use page flag interfaces when copying a grant page
  2010-02-23 17:24       ` [PATCH] grant-table: use page flag interfaces when copying a grant page Ian Campbell
@ 2010-02-23 17:25         ` Ian Campbell
  0 siblings, 0 replies; 8+ messages in thread
From: Ian Campbell @ 2010-02-23 17:25 UTC (permalink / raw)
  To: xen-devel; +Cc: Jeremy Fitzhardinge, Jan Beulich

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(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [GIT] Grant table fix
  2010-02-23 16:39 [GIT] Grant table fix Ian Campbell
  2010-02-23 16:40 ` [PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page Ian Campbell
@ 2010-02-23 18:02 ` Jeremy Fitzhardinge
  2010-02-23 18:05   ` Ian Campbell
  1 sibling, 1 reply; 8+ messages in thread
From: Jeremy Fitzhardinge @ 2010-02-23 18:02 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [GIT] Grant table fix
  2010-02-23 18:02 ` [GIT] Grant table fix Jeremy Fitzhardinge
@ 2010-02-23 18:05   ` Ian Campbell
  0 siblings, 0 replies; 8+ messages in thread
From: Ian Campbell @ 2010-02-23 18:05 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: xen-devel

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-02-23 18:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-23 16:39 [GIT] Grant table fix Ian Campbell
2010-02-23 16:40 ` [PATCH] gnttab: propagate Reserved flag from old to new page in gnttab_copy_grant_page Ian Campbell
2010-02-23 17:04   ` Jan Beulich
2010-02-23 17:08     ` Ian Campbell
2010-02-23 17:24       ` [PATCH] grant-table: use page flag interfaces when copying a grant page Ian Campbell
2010-02-23 17:25         ` Ian Campbell
2010-02-23 18:02 ` [GIT] Grant table fix Jeremy Fitzhardinge
2010-02-23 18:05   ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.