Thanks for tracking this down and supplying these patches. I'm confused by a couple of things though:
- For base.cc, how was there a leak in the original code? I agree it's pretty contorted; the one place where keep_trying used to be sometimes set to true got deleted, see http://repo.m5sim.org/m5/rev/874f2ee2f115, and the code never got restructured to reflect the fact that keep_trying is always false. Given that keep_trying is always false in the original code though I don't see a memory leak, since we always just return the packet we pop. Your patch actually changes the behavior of the prefetcher pretty significantly (sort of emulating the old !cacheCheckPush behavior). - The stride.cc fix makes sense, but isn't it adequate to just add: delete *min_pos; in front of tab.erase(min_pos) without adding the min_entry variable? Steve On Wed, Jun 2, 2010 at 10:37 AM, Ankit Sethia <[email protected]> wrote: > Hi, > > After running valgrind i found the leaks. I have fixed them locally and my > applications dont blow up in memory after the fix. Here is the diff, kindly > let me know if it is correct: > > diff -r 84bd4089958b src/mem/cache/prefetch/base.cc > --- a/src/mem/cache/prefetch/base.cc Tue May 25 20:15:44 2010 -0700 > +++ b/src/mem/cache/prefetch/base.cc Wed Jun 02 13:21:46 2010 -0400 > @@ -138,23 +138,25 @@ > } > > PacketPtr pkt; > - bool keep_trying = false; > + bool keep_trying = true; > do { > pkt = *pf.begin(); > pf.pop_front(); > > - if (keep_trying) { > + if ((pkt != (*pf.end())) && inCache(pkt->getAddr())) { > + keep_trying = true; > DPRINTF(HWPrefetch, "addr 0x%x in cache, skipping\n", > pkt->getAddr()); > delete pkt->req; > delete pkt; > } > + else { > + keep_trying = false; > + } > > - if (pf.empty()) { > + if (keep_trying && pf.empty()) { > cache->deassertMemSideBusRequest(BaseCache::Request_PF); > - if (keep_trying) { > - return NULL; // None left, all were in cache > - } > + return NULL; // None left, all were in cache > } > } while (keep_trying); > > diff -r 84bd4089958b src/mem/cache/prefetch/stride.cc > --- a/src/mem/cache/prefetch/stride.cc Tue May 25 20:15:44 2010 -0700 > +++ b/src/mem/cache/prefetch/stride.cc Wed Jun 02 13:21:46 2010 -0400 > @@ -110,13 +110,16 @@ > if (tab.size() >= 256) { //set default table size is 256 > std::list<StrideEntry*>::iterator min_pos = tab.begin(); > int min_conf = (*min_pos)->confidence; > + StrideEntry* min_entry = NULL; > for (iter = min_pos, ++iter; iter != tab.end(); ++iter) { > if ((*iter)->confidence < min_conf){ > min_pos = iter; > min_conf = (*iter)->confidence; > + min_entry = *iter; > } > } > DPRINTF(HWPrefetch, " replacing PC %x\n", > (*min_pos)->instAddr); > + delete min_entry; > tab.erase(min_pos); > } > > On Tue, Jun 1, 2010 at 7:46 PM, Korey Sewell <[email protected]> wrote: >> >> Has anyone come across such an issue or can any one suggest me what is >> the appropriate way of finding out where the problem lies? >> Try using "valgrind" to check where the memory leak is coming from. It's a >> tremendously useful tool. >> >> you'll want to run on a debug binary and for a limited number of cycles as >> well.. >> >> a command line such as "valgrind --tool=memcheck --leak-check=yes >> <cmd_line>" usually works for me. (it's a lot of output, be warned). >> >> >>> >>> The GHB, prefetching mechanism works fine in all cases. >>> >>> Another problem that i am currently facing is as follow: >>> There are only integers and floating point numbers in the application >>> that i am trying to run. >> >> Check the mailing list archives but I'm not sure Floating Point support is >> all the way there for x86. I'll let someone else chime in. >> >> >>> >>> build/X86_SE/cpu/simple/timing.cc:438: Fault TimingSimpleCPU::read(Addr, >>> T&, unsigned int) [with T = uint64_t]: Assertion `split_addr <= addr || >>> split_addr - addr < block_size' failed. >> >> That looks like a error in the code trying to determine something with >> split accesses (accesses that span multiple cache blocks)... >> >> It could be something to do with your prefetcher...Or it could be >> something with the memory leak corrupting values. >> >> >> >> -- >> - Korey >> >> _______________________________________________ >> m5-users mailing list >> [email protected] >> http://m5sim.org/cgi-bin/mailman/listinfo/m5-users > > > > -- > Ankit > > _______________________________________________ > m5-users mailing list > [email protected] > http://m5sim.org/cgi-bin/mailman/listinfo/m5-users > _______________________________________________ m5-users mailing list [email protected] http://m5sim.org/cgi-bin/mailman/listinfo/m5-users
