> On Sept. 2, 2015, 2:02 p.m., Jason Power wrote: > > Is it not possible to delete it when the entry is set to NotPresent? It > > seems weird to delete it in the allocate function. > > Nilay Vaish wrote: > I think the problem here is with the coherence protocols maintaining two > invalid states: NP and I. > NP is supposed to be the state when the entry has not been allocated, > while in I state, the entry > is allocated but is invalid. I think this was supposed to be an > optimization that helps in avoiding > memory allocation and deallocation. > > Joel, if whatever I have said above is right, I would suggest that you > eliminate either of the states and > always allocate and deallocate entries. I think the greater problem is > SLICC's lack of constructors.
Nilay's clarification on the original intent is correct. The problem is that the interface for allocating entries is too loose: A specific problem here is that the cache entry constructor sets permissions to NotPresent, and the allocation can occur anywhere in a .sm file (this is a memory management issue exposed to the programmer :/). As a result, there's no way of knowing how a protocol author might use the entry (e.g. placing an entry in a TBE, and later moving it into the cache, and the permission may not get updated). This allows the cache to contain allocated, NotPresent lines that can be leaked in allocate(). We'd need to make a much broader change to address sources of this leak, and that could affect a lot of downstream code. I've validated this simple fix, and it will work with any downstream code. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3087/#review7090 ----------------------------------------------------------- On Sept. 2, 2015, 1:46 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3087/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2015, 1:46 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11078:30183e60a824 > --------------------------- > ruby: Fix CacheMemory allocate leak > > If a cache entry permission was previously set to NotPresent, but the entry > was > not deleted, a following cache allocation can cause the entry to be leaked by > setting the entry pointer to a newly allocated entry. To eliminate this > possibility, check if the new entry is different from the old one, and if so, > delete the old one. > > > Diffs > ----- > > src/mem/ruby/structures/CacheMemory.cc 2763a59c73ff > > Diff: http://reviews.gem5.org/r/3087/diff/ > > > Testing > ------- > > > Thanks, > > Joel Hestness > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
