Ok, so things are somewhat calm, and I'm trying to take time off to see what's going on. And I'm not happy. On Mon, Nov 10, 2014 at 10:28 AM, <j.glisse@xxxxxxxxx> wrote: > > Page table is a common structure format most notably use by cpu mmu. The > arch depend page table code has strong tie to the architecture which makes > it unsuitable to be use by other non arch specific code. Please don't call this thing a "generic page table". It is no such thing. The *real* page tables are page tables. This is some kind of "mapping lookup", and has nothing to do with page tables as far as I can see. Why do you call it a page table? Also, why isn't this just using our *existing* generic mapping functionality, which already uses a radix tree, and has a lot of lockless models? We already *have* something like that, and it's called a "struct address_space". And if you *just* want the tree, why don't you use "struct radix_tree_root". And if it's generic, why do you have that odd insane conditional locking going on? In other words, looking at this, I just go "this is re-implementing existing models, and uses naming that is actively misleading". I think it's actively horrible, in other words. The fact that you have one ACK on it already makes me go "Hmm". Is there some actual reason why this would be called a page table, when even your explanation very much clarifies that it is explicitly written to *not* be an actual page table. I also find it absolutely disgusting how you use USE_SPLIT_PTE_PTLOCKS for this, which seems to make absolutely zero sense. So you're sharing the config with the *real* page tables for no reason I can see. I'm also looking at the "locking". It's insane. It's wrong, and doesn't have any serialization. Using the bit operations for locking is not correct. We've gotten over that years ago. Rik, the fact that you acked this just makes all your other ack's be suspect. Did you do it just because it was from Red Hat, or do you do it because you like seeing Acked-by's with your name? Anyway, this gets a NAK from me. Maybe I'm missing something, but I think naming is supremely important, and I really don't see the point of this. At a minimum, it needs a *hell* of a lot more explanations for all it does. And quite frankly, I don't think that will be sufficient, since the whole "bitops for locking" looks downright buggy, and it's not at all clear why you want this in the first place as opposed to just using gang lookups on the radix trees that we already have, and that is well-tested and known to scale fine. So really, it boils down to: why is this any better than radix trees that are well-named, tested, and work? Linus -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>