> On April 17, 2012, 7:44 a.m., Steve Reinhardt wrote: > > I'm a little confused... are you saying that we could initiate a page table > > walk for a page where there's already a walk outstanding? Under what > > circumstances does this come up? > > > > Wouldn't it be easier and make more sense just to suppress the later > > response than to delete an existing entry? > > > > Assuming this was causing problems before, is there a place we should add > > an assertion to catch future similar bugs (like someone implementing the > > TLB in another ISA trying to overwrite an existing entry)? > > Gabe Black wrote: > Yes, because the TLB can initiate multiple walks in parallel. Until it > looks up the entry for a virtual address it doesn't know how big the page is, > so it doesn't know what would collide ahead of time. The bug was that we were > hitting an assert, so there's already one there.
OK, I suspected the multiple page size issue could be a problem. Do we bother to suppress parallel walks that lie within the same minimum-sized page? Seems like that would be reasonable. It's strictly an optimization, but it's one that could impact both performance and performance modeling accuracy. Glad to hear about the assert. I know that at this point in time it's easier to leave your existing code in than to change it, but it still seems to me that doing a lookup when a walk completes and discarding the new entry if there's a hit is logically simpler than replacing the existing entry. Functionally it shouldn't matter; the OS should flush the TLB if the mapping changes, so I don't see a legitimate scenario in which the two walks wouldn't return the same PTE anyway. You could even assert that if you were feeling paranoid. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1155/#review2553 ----------------------------------------------------------- On April 17, 2012, 5:45 a.m., Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1155/ > ----------------------------------------------------------- > > (Updated April 17, 2012, 5:45 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 8956:ce879248b675 > --------------------------- > X86: Clear out duplicate TLB entries when adding a new one. > > It's possible for two page table walks to overlap which will go in the same > place in the TLB's trie. They would land on top of each other, so this change > adds some code which clears out any potentially conflicting entries before > adding a new one. > > > Diffs > ----- > > src/arch/x86/tlb.cc bbceb6297329 > > Diff: http://reviews.gem5.org/r/1155/diff/ > > > Testing > ------- > > > Thanks, > > Gabe Black > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
