> On April 8, 2012, 5:20 p.m., Steve Reinhardt wrote: > > src/arch/x86/tlb.cc, line 84 > > <http://reviews.gem5.org/r/1143/diff/1/?file=25687#file25687line84> > > > > This whole algorithm here is (1) a bit inscrutable and (2) looks > > potentially inefficient. (1) could be fixed with some comments. Should I > > be concerned about (2)?
Untrue. I have scruted it myself. The old algorithm had one nice property in that the list of TLB entries were always in LRU order. If you needed to free something, you'd just grab the last thing in the list which was automatically the oldest, since everything else would have more recently been moved to the head of the list when it was used. With this new scheme, I decided to instead just write an incrementing integer to each entry as it was accessed which is very easy. Then, in the relatively rare even that there was an eviction, you need to look through all the entries and find the one with the lowest sequence number. I use whether or not there's a trie node associated with the entry as a valid bit. I'm sure there are ways it can be more efficient, but this way isn't that bad and evictions should be rare. Finding the min of a small set of entries is also pretty quick compared to going through the whole page table walker mechanism to find the new entry. > On April 8, 2012, 5:20 p.m., Steve Reinhardt wrote: > > src/arch/x86/vtophys.cc, line 64 > > <http://reviews.gem5.org/r/1143/diff/1/?file=25688#file25688line64> > > > > same comment here > > Steve Reinhardt wrote: > referring back to an earlier comment that somehow reviewboard dropped on > me... the comment was attached to the earlier "Addr width", and said that (1) > I doubt Addr is still the most appropriate type here and (2) "width" is not > the most descriptive name either. How about pageSizeBits or shiftBits or > something like that? How about making it an unsigned and calling it logBytes? I'll implement that. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1143/#review2509 ----------------------------------------------------------- On April 8, 2012, 1:02 a.m., Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1143/ > ----------------------------------------------------------- > > (Updated April 8, 2012, 1:02 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 8945:f40e80105a03 > --------------------------- > X86: Use the AddrTrie class to implement the TLB. > > This change also adjusts the TlbEntry class so that it stores the number of > address bits wide a page is rather than its size in bytes. In other words, > instead of storing 4K for a 4K page, it stores 12. 12 is easy to turn into 4K, > but it's a little harder going the other way. > > > Diffs > ----- > > src/arch/x86/pagetable.hh a47fd7c2d44e > src/arch/x86/pagetable.cc a47fd7c2d44e > src/arch/x86/pagetable_walker.hh a47fd7c2d44e > src/arch/x86/pagetable_walker.cc a47fd7c2d44e > src/arch/x86/tlb.hh a47fd7c2d44e > src/arch/x86/tlb.cc a47fd7c2d44e > src/arch/x86/vtophys.cc a47fd7c2d44e > > Diff: http://reviews.gem5.org/r/1143/diff/ > > > Testing > ------- > > > Thanks, > > Gabe Black > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
