All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
To: Sander Eikelenboom <linux@eikelenboom.it>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Ben Guthro <ben@guthro.net>,
	Robert Phillips <robert.phillips@citrix.com>,
	Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
Subject: Re: dom0 linux 3.6.0-rc4, crash due to ballooning althoug dom0_mem=X, max:X set
Date: Wed, 12 Sep 2012 12:28:28 +0100	[thread overview]
Message-ID: <alpine.DEB.2.02.1209121228090.15568@kaball.uk.xensource.com> (raw)
In-Reply-To: <1901811929.20120912122848@eikelenboom.it>

On Wed, 12 Sep 2012, Sander Eikelenboom wrote:
> Tuesday, September 11, 2012, 6:02:47 PM, you wrote:
> 
> > On Wed, 5 Sep 2012, Konrad Rzeszutek Wilk wrote:
> >> On Tue, Sep 04, 2012 at 04:27:20PM -0400, Robert Phillips wrote:
> >> > Ben,
> >> > 
> >> > You have asked me to provide the rationale behind the gnttab_old_mfn patch, which you emailed to Sander earlier today. 
> >> > Here are my findings.
> >> > 
> >> > I found that xen_blkbk_map() in drivers/block/xen-blkback/blkback.c has changed from our previous version.  It now calls gnttab_map_refs() in drivers/xen/grant-table.c.
> >> > 
> >> > That function first calls HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, ... ) and then calls m2p_add_override() in p2m.c
> >> 
> >> And HYPERVISOR_grant_table_op .. would populate map_ops[i].bus_addr with the machine address..
> >> 
> >> > which is where I made my change.
> >> > 
> >> > The unpatched code was saving the pfn's old mfn in kmap_op->dev_bus_addr.  
> >> > 
> >> > kmap_op is of type struct gnttab_map_grant_ref.  That data type is used to record grant table mappings so later they can be unmapped correctly.
> >> 
> >> Right, but the blkback makes a distinction by passing NULL as kmap_op, which means it should
> >> use the old mechanism. Meaning that once the hypercall is done, the map_ops[i].bus_addr is not
> >> used anymore..
> >> 
> >> > 
> >> > The problem with saving the old mfn in kmap_op->dev_bus_addr is that it is later overwritten by __gnttab_map_grant_ref() in xen/common/grant_table.c
> >> 
> >> Uh, so the problem of saving the old mfn in dev_bus_addr has been there for a long long time then?
> >> Even before this patch set?
> 
> > I think that Robert identified the real problem: dev_bus_addr shouldn't
> > have been used here. However the bug only shows up if we are batching
> > the grant table operations, that we started doing since
> > f62805f1f30a40e354bd036b4cb799863a39be4b.
> > That's why Sander's bisection found that
> > f62805f1f30a40e354bd036b4cb799863a39be4b is the culprit.
> 
> > However the fix is incorrect because it is modifying a struct that is
> > part of the Xen ABI.
> > I am appending an alternative fix that doesn't need any changes to
> > public headers.
> 
> > Sander, could you please let me know if it fixes the problem for you?
> 
> It does !
> 
> Tested-By: Sander Eikelenboom <linux@eikelenboom.it>
> 

Thanks for testing!

  reply	other threads:[~2012-09-12 11:28 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-04 16:37 dom0 linux 3.6.0-rc4, crash due to ballooning althoug dom0_mem=X, max:X set Sander Eikelenboom
2012-09-04 16:33 ` Konrad Rzeszutek Wilk
2012-09-04 17:19   ` Sander Eikelenboom
2012-09-04 18:07     ` Ben Guthro
2012-09-04 18:22       ` Konrad Rzeszutek Wilk
2012-09-04 18:57         ` Sander Eikelenboom
2012-09-04 19:34       ` Sander Eikelenboom
2012-09-04 20:27         ` Robert Phillips
2012-09-05 14:06           ` Konrad Rzeszutek Wilk
2012-09-05 14:38             ` Sander Eikelenboom
2012-09-05 20:19               ` Konrad Rzeszutek Wilk
2012-09-05 22:52                 ` Sander Eikelenboom
2012-09-06 10:57                   ` Konrad Rzeszutek Wilk
2012-09-06 11:16                     ` Sander Eikelenboom
2012-09-06 16:46                     ` Sander Eikelenboom
2012-09-11 16:02             ` Stefano Stabellini
2012-09-12 10:28               ` Sander Eikelenboom
2012-09-12 11:28                 ` Stefano Stabellini [this message]
2012-09-13 13:32                 ` Konrad Rzeszutek Wilk
2012-09-13 13:42                   ` Robert Phillips
2012-09-14 14:53                   ` Conny Seidel
2012-09-14 17:00                     ` Konrad Rzeszutek Wilk
2012-09-14 17:38                       ` Conny Seidel
2012-09-17 19:14                   ` Sander Eikelenboom
2012-09-17 19:23                     ` Konrad Rzeszutek Wilk
2012-09-04 16:39 ` Konrad Rzeszutek Wilk
2012-09-04 18:02   ` Sander Eikelenboom
2012-09-04 17:58     ` Konrad Rzeszutek Wilk
2012-09-04 19:01       ` Sander Eikelenboom
2012-09-04 20:13       ` Sander Eikelenboom
2012-09-04 21:23       ` Sander Eikelenboom

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.02.1209121228090.15568@kaball.uk.xensource.com \
    --to=stefano.stabellini@eu.citrix.com \
    --cc=ben@guthro.net \
    --cc=konrad.wilk@oracle.com \
    --cc=linux@eikelenboom.it \
    --cc=robert.phillips@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.