Re: [PATCH] libxl: introduce an option for disabling the non-O_DIRECT workaround

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Nov 26, 2013 at 05:56:54PM +0000, George Dunlap wrote:
> On 11/26/2013 05:46 PM, Konrad Rzeszutek Wilk wrote:
> >On Tue, Nov 26, 2013 at 11:42:31AM +0000, Ian Jackson wrote:
> >>From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>
> >>Document and implement a new option that permits disk backends which
> >>would otherwise have to avoid O_DIRECT (because of the network memory
> >>lifetime bug) to use it anyway.  This is:
> >
> >I thought this was fixed in the Linux kernel with:
> >
> >commit cd9151e26d31048b2b5e00fd02e110e07d2200c9
> >Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >Date:   Sun Aug 4 15:39:40 2013 +0100
> >
> >     xen/balloon: set a mapping for ballooned out pages
> >
> >     Currently ballooned out pages are mapped to 0 and have INVALID_P2M_ENTRY
> >     in the p2m. These ballooned out pages are used to map foreign grants
> >     by gntdev and blkback (see alloc_xenballooned_pages).
> >
> >     Allocate a page per cpu and map all the ballooned out pages to the
> >     corresponding mfn. Set the p2m accordingly. This way reading from a
> >     ballooned out page won't cause a kernel crash (see
> >     http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html).
> >
> >
> >?
> 
> Not everyone builds her own kernel from the latest release; until we
> can be relatively sure that this fix has hit distros (including
> older LTS-style ones), we have to deal with the fact that the
> O_DIRECT bug may be present.  The purpose of this flag is to enable
> you to turn it on when you know it's safe -- for instance, if you're
> running a kernel with this changeset.

Of course. I think it might be worth referecing this commit in this
patchset.
> 
> It is worth asking the question now though: Given that this has been
> checked in, would it make sense to switch the polarity of this --
> default to O_DIRECT on and have a flag to allow people to switch it
> off?  If not now, maybe for 4.5?  And in that case, maybe we should
> change the argument to be tristate: default, true, or false (i.e.,
> true means I know it's safe, false means I know it's unsafe, default
> means I haven't specified either way)?

Perhaps the bug reporter (CC-ed here) could chime in with which
he/she feels is better?

> 
>  -George
>