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

[Linus Torvalds <torvalds@osdl.org>]

On Tue, 25 May 2004, Matthew Wilcox wrote:

> On Mon, May 24, 2004 at 09:00:02PM -0700, Linus Torvalds wrote:
> > I suspect we should just make a "ptep_set_bits()" inline function that 
> > _atomically_ does "set the dirty/accessed bits". On x86, it would be a 
> > simple
> > 
> > 		asm("lock ; orl %1,%0"
> > 			:"m" (*ptep)
> > 			:"r" (entry));
> > 
> > and similarly on most other architectures it should be quite easy to do 
> > the equivalent. You can always do it with a simple compare-and-exchange 
> > loop, something any SMP-capable architecture should have.
> 
> ... but PA doesn't.  Just load-and-clear-word (and its 64-bit equivalent
> in 64-bit mode).  And that word has to be 16-byte aligned.

Wow. And this architecture claims to support SMP? 

> What race are we protecting against?  If it's like xchg() and we only
> need to protect against a racing xchg() and not a reader, we can just
> reuse the global array of hashed spinlocks we have for that.

The race is:
 - one CPU sets the dirty bit (possibly with a hardware walker, but I 
   guess on PA it's probably done in sw)
 - the other CPU sets the accessed bit in sw as part of the 
   "handle_pte_fault()" processing.

Right now we set the accessed bit with a simple "ptep_establish()", which 
will use "set_pte()", which is just a regular write. So setting the 
accessed bit will basically be a nonatomic sequence of

 - read pte entry
 - entry = pte_mkyoung(entry)
 - set_pte(entry)

which is all done under the mm->page_table_lock, but which does NOT 
protect against any hardware page-table walkers or any asynchronous sw 
walkers (if anybody does them).

Basically, the suggestion is to replace the "set_pte()" with something 
that is safe against anything else that updates the page tables (whether 
software or hardware). If only core kernel code does that, then you should 
already be fine, since the page-table spinlock should already be held by 
all updaters.

NOTE! One really easy approach would be to say that we never mix software 
updates of the accessed bit with hw updates, and just have a rule that if 
the architecture does accessed-bit updates in hardware (and can thus race 
with us doing them in software _despite_ the fact that we hold the page 
table lock), then we just don't do the update at all. 

We'd just pass in a flag to "ptep_establish()" to tell it whether we
changed the dirty bit or not. It would be "write_access" in
handle_pte_fault(), and 1 in the other two cases.

> Ah, atomic writes we can do.  That's easy.  I think all Linux architectures
> support atomic writes to naturally aligned addresses, don't they?

Yes. You'd really have to work at it _not_ to support them ;)

However, the atomic write case only helps in the case when we update _all_ 
the bits that hw walkers can update, 

			Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>