William Lee Irwin III wrote: > On Fri, Oct 29, 2004 at 09:45:57PM +1000, Nick Piggin wrote: > >>One more patch - this provides a generic framework for pte >>locks, and a basic i386 reference implementation (which just >>ifdefs out the cmpxchg version). Boots, runs, and has taken >>some stressing. >>I should have sorted this out before sending the patches for >>RFC. The generic code actually did need a few lines of changes, >>but not much as you can see. Needs some tidying up though, but >>I only just wrote it in a few minutes. >>And now before anyone gets a chance to shoot down the whole thing, >>I just have to say >> "look ma, no page_table_lock!" > > > The large major problem to address is making sure this works with > arches. Without actually examining the arches this needs to be made to > work with, it's not any kind of advance. > Well it is because we've now got 3 synchronisation schemes that arches can use. And I just demonstrated that all 3 (locked, cmpxchg, pte-locked) work on i386. So it is a long way off from saying N architectures _do_ work, but the possibility is there. > The only way to demonstrate that the generic API is any kind of > progress toward that end is to sweep the arches and make them work. > > So, the claim of "look ma, no page_table_lock" is meaningless, as no > arches but x86(-64) have been examined, audited, etc. The most disturbing > of these is the changing of the locking surrounding tlb_finish_mmu() et > al. It's not valid to decouple the locking surrounding tlb_finish_mmu() > from pagetable updates without teaching the architecture-specific code > how to cope with this. > i386 looks OK. I admit that this is an area I haven't looked deeply into yet, but the synchronisation there is a bit soft anyway, because you can have other threads scheduling onto other CPUs at any time, and it has to be able to cope with that. All except sparc64 by the looks, which takes the page table lock when context switching (which is fairly interesting). > It's also relatively sleazy to drop this in as an enhancement for just > a few architectures (x86[-64], ia64, ppc64), and leave the others cold, > but I won't press that issue so long as the remainder are functional, > regardless of my own personal preferences. > OK, it will work for all the arches that Christoph's works for, so that's i386, x86-64, ia64, s390. We're _hoping_ that it will work for ppc64 (and if I had a ppc64 I would have at least made some attempts by now). Seriously, what is other arch would want this? I'm not saying no others could be done, but there is very little point IMO. > What is unacceptable is the lack of research into the needs of arches > that has been put into this. The general core changes proposed can > never be adequate without a corresponding sweep of architecture- > specific code. While I fully endorse the concept of lockless pagetable > updates, there can be no correct implementation leaving architecture- > specific code unswept. I would encourage whoever cares to pursue this > to its logical conclusion to do the necessary reading, and audits, and > review of architecture manuals instead of designing core API's in vacuums. > Definitely - which is one of the reasons I posted it here, because I don't pretend to know all the arch details. But if you think I designed it in a vacuum you're wrong. > I'm sorry if it sounds harsh, but I can't leave it unsaid. I've had to > spend far too much time cleaning up after core changes carried out in > similar obliviousness to the needs of architectures already, and it's > furthermore unclear that I can even accomplish a recovery of a > significant number of architectures from nonfunctionality in the face > of an incorrect patch of this kind without backing it out entirely. > Some of the burden of proof has to rest on he who makes the change; > it's not even necessarily feasible to break arches with a patch of this > kind and accomplish any kind of recovery of a significant number of them. > Bill, I'm not suggesting this gets merged as is. Maybe the lack of an [RFC] in the subject threw you off. _If_ it were to ever get merged, it would be only after it was shown to work on all architectures. And no, I'm really not asking you to do anything at all; not even on sparc because that should work with basically zero changes. Something like this should get sparc64 working again.