On Mon, Oct 22, 2007 at 09:07:40PM +0200, Andi Kleen wrote:
> On Mon, Oct 22, 2007 at 11:40:52AM -0700, Jeremy Fitzhardinge wrote:
> > Andi Kleen wrote:
> > > Jeremy Fitzhardinge <jeremy@xxxxxxxx> writes:
> > >
> > >> Yes, that's precisely the problem. xfs does delay the unmap, leaving
> > >> stray mappings, which upsets Xen.
> > >>
> > >
> > > Again it not just upsets Xen, keeping mappings to freed pages is wrong
> > > generally
> > > and violates the x86 (and likely others like PPC) architecture because it
> > > can
> > > cause illegal caching attribute aliases.
> > >
> > > The patch that went into the tree was really not correct -- this
> > > bogus optimization should have been unconditionally removed
> > > or if you really wanted an ifdef made dependent on !CONFIG_XEN &&
> > > !CONFIG_AGP (and likely && !CONFIG_DRM && !CONFIG_anything else that
> > > uses uncached mappings in memory).
> > >
> > > You just worked around the obvious failure and leave the non obvious
> > > rare corruptions in, which isn't a good strategy.
> > Well, at least it becomes a known issue and/or placeholder for when Nick
> It's hidden now so it causes any obvious failures any more. Just
> subtle ones which is much worse.
There's no evidence of any problems ever being caused by this code.
It's been there for years.
> But why not just disable it? It's not critical functionality,
> just a optimization that unfortunately turned out to be incorrect.
It is critical functionality for larger machines because vmap()/vunmap()
really does suck in terms of scalability.
> It could be made correct short term by not freeing the pages until
> vunmap for example.
Could vmap()/vunmap() take references to the pages that are
mapped? That way delaying the unmap would also delay the freeing
of the pages and hence we'd have no problems with the pages
being reused before the mapping is torn down. That'd work for
Xen even with XFS's lazy unmapping scheme, and would allow
Nick's more advanced methods to work as well....
SGI Australian Software Group