Re: [PATCH] [ARM] force dcache flush if dcache_dirty bit set

From: Hugh Dickins
Date: Mon Oct 12 2009 - 14:07:55 EST


On Mon, 12 Oct 2009, Russell King wrote:
> On Mon, Oct 12, 2009 at 05:09:53PM +0100, Hugh Dickins wrote:
> > Sorry to muddy the waters on this, if you and Dave are sure that
> > you have the right fix, down in your architectures, and that fix
> > isn't going to hurt your performance significantly.
>
> If I look at the issue from this point of view:
>
> - we are using PG_arch_1 to delay cache handling for the page
>
> - if PG_arch_1 is set on a page, we set it explicitly because we
> didn't do some flushing between the allocation of the page and
> mapping it into userspace
>
> - if a page with PG_arch_1 set ever gets to userspace, this can
> only be because we did the lazy flushing thing
>
> I don't see that there should have been any bearing on whether a page
> has a mapping or not when we get to update_mmu_cache. The issue here
> is that > if PG_arch_1 is set on a page, then we didn't flush it at
> the time when we believed it was appropriate to do so. <
>
> Tell me I'm wrong (having only just sent it to Linus...)

I wouldn't dare! Put that way, it seems very clear that it was always
at best a redundant test, which Nitin now shows can go wrong. Okay,
thanks, let's forget about whether file invalidation can or cannot
also put a page into this state, and proceed with your fix.

The architectures which appear to need fixing in the same way are
arm, mips, parisc, sh and sparc64 (xtensa looks right already, and
score just looks confused - a whole function __update_cache() which
checks for PG_arch_1, yet nothing sets it?).

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/