Re: [PATCH] ppc64: Fix possible race with set_pte on a present PTE

Benjamin Herrenschmidt
2004-05-24T05:34:49Z

> Ahh.. That's a bug, methinks.
>
> The reason it's a bug is that if you do this, you can lose the dirty bit
> being written on some other CPU asynchronously.

Hrm... right indeed.

> In other words, I think it's pretty much always a bug to do a "set_pte()"
> with an existing pte in place, exactly because you lose information. You
> are trying to cover up the bug in ppc64-specific code, but I think that
> what you found is actually a (really really) unlikely race condition that
> can have serious consequences.
>
> Or am I missing something else?

Well, the original scenario triggering that from userland is, imho, so
broken, that we may just not care losing that dirty bit ... Oh well :)
Anyway, apply my patch. If pte is not present, this will have no effect,
if it is, it makes sure we never leave a stale HPTE in the hash, which
is fatal in far worse ways.

> [ grep grep grep ]
>
> Looks like "break_cow()" and "do_wp_page()" are safe, but only because
> they always sets the dirty bit, and any other bits end up being pretty
> much "don't care if we miss an accessed bit update" or something.
>
> Hmm. Maybe I'm wrong. If this really is buggy, it's been buggy this way
> basically forever. That code is _not_ new, it's some of the oldes code in
> the whole VM since the original three-level code rewrite. I think. Of
> course, back then SMP wasn't an issue, and this seems to have survived all
> the SMP fixes.
>
> Who else has been working on the page tables that could verify this for
> me? Ingo? Ben LaHaise? I forget who even worked on this, because it's so
> long ago we went through all the atomicity issues with the page table
> updates on SMP. There may be some reason that I'm overlooking that
> explains why I'm full of sh*t.

Ben.