Re: [gem5-dev] Review Request 2624: mem: mmap the backing store with MAP_NORESERVE
On Feb. 3, 2015, 9:37 p.m., Brandon Potter wrote: The patch seems like a landmine for an unsuspecting user as it would be difficult to diagnose if swap space is exhausted. It will probably be evident that memory exhaustion caused the runtime error (segmentation fault), but tracking down the cause to this patch will probably be non-trivial (maybe?). For a user, the memory allocation failure from insufficient swap space would probably be preferable to a hard to diagnose segmentation fault. Even though it is ugly, maybe it's best to add #if 0 / #endif around the code to allow someone to find this later on and enable the feature if they need it (with a warning about the segmentation fault). The memory allocation failure will point the user to the code in this patch. Andreas Hansson wrote: Fair point. An option would be to not use the flag for sensible sizes, perhaps 16 GB or so, and adopt it for larger sizes with a warning? Brandon Potter wrote: Perhaps that's fine. Although, I think that a value for sensible will vary from person to person. Personally, I still feel that it is better to leave it completely disabled and allow someone to enable it if needed (solely to prevent confusion later). It is trivial to enable the functionality by modifying the condition. Another avenue might be whether it is worthwhile to use a preprocessor define and the #ifdef to enable/disable this during compilation; might be overkill to go through the trouble though. Overall, maybe this is just paranoia on my part; it does seem unlikely to cause a problem in practice. Andreas Hansson wrote: The input is much appreciated. My hesitation to the approach of asking the user to enable it if needed, is that it means yet another parameter to keep track of (or yet another patch to manage). If we go for the limit approach, and a large value like 1 TB perhaps? I also think this is unlikely to cause problems the way it is, and as such I would prefer not adding more configurability unless you really think it is needed. Brandon Potter wrote: Seems reasonable. There is a tradeoff either way. Andreas Hansson wrote: So what do people think? 1) Good as it is (avoid more switches and magic limits)? 2) Add a limit, e.g. 1 TB and use above that? 3) Add a switch and force the user to explicitly set it? In the spirit of keeping things simple I'd suggest (1). Steve Reinhardt wrote: Hi Andreas, I'm curious about the scenarios you have where you want to simulate a system with TB of physical memory but don't expect to touch more than a few GB of that. That seems like a special case to me. I sympathize with Brandon's point, and I don't want to make things more confusing for what I would guess is the majority of users (in terms of them getting weird delayed faults when they could have been told up front they need more swap) just to make the default case work for what I would guess is a minority of users who probably know they're unusual by trying to simulate really large memories. My inclination would be to make the use of MAP_NORESERVE for the backing store a SimObject option that's false by default (maintaining the current behavior). At AMD, we end up writing custom top-level python scripts for our major configurations anyway, so if we wanted to hardwire that to true for some of those configs, we could just do that in our custom script. If, in some configurations, people really want to define a threshold to do this automatically, then that can be done in python. Or to put it another way, I definitely want to enable you to use MAP_NORESERVE without having to maintain it as a patch on the gem5 tree, but since I haven't heard any clamoring from the broader user population for this change (did I miss it?), I'm not too enthusiastic about enabling it by default. I'll add a switch. This patch allows a sparse address map without actually implementing a sparse memory in gem5 and adding another level of indirection. This patch simply leaves it to the OS to allocate only those pages we touch, and does not complain if they are very far apart in the physical address space. - Andreas --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2624/#review5821 --- On Feb. 3, 2015, 7:57 p.m., Andreas Hansson wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2624/ --- (Updated Feb. 3, 2015, 7:57 p.m.) Review request for Default. Repository: gem5 Description ---
Re: [gem5-dev] Review Request 2646: mem: Split port retry for all different packet classes
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2646/ --- (Updated Feb. 8, 2015, 5:48 p.m.) Review request for Default. Repository: gem5 Description (updated) --- Changeset 10705:1fec273b2946 --- mem: Split port retry for all different packet classes This patch fixes a long-standing isue with the port flow control. Before this patch the retry mechanism was shared between all different packet classes. As a result, a snoop response could get stuck behind a request waiting for a retry, even if the send/recv functions were split. This caused message-dependent deadlocks in stress-test scenarios. The patch splits the retry into one per packet (message) class. Thus, sendTimingReq has a corresponding recvReqRetry, sendTimingResp has recvRespRetry etc. Most of the changes to the code involve simply clarifying what type of request a specific object was accepting. The biggest change in functionality is in the cache downstream packet queue, facing the memory. This queue was shared by requests and snoop responses, and it is now split into two queues, each with their own flow control, but the same physical MasterPort. These changes fixes the previously seen deadlocks. Diffs (updated) - src/arch/x86/pagetable_walker.hh 94901e131a7f src/arch/x86/pagetable_walker.cc 94901e131a7f src/cpu/kvm/base.hh 94901e131a7f src/cpu/minor/fetch1.hh 94901e131a7f src/cpu/minor/fetch1.cc 94901e131a7f src/cpu/minor/lsq.hh 94901e131a7f src/cpu/minor/lsq.cc 94901e131a7f src/cpu/o3/cpu.hh 94901e131a7f src/cpu/o3/cpu.cc 94901e131a7f src/cpu/o3/fetch.hh 94901e131a7f src/cpu/o3/fetch_impl.hh 94901e131a7f src/cpu/o3/lsq.hh 94901e131a7f src/cpu/o3/lsq_impl.hh 94901e131a7f src/cpu/simple/atomic.hh 94901e131a7f src/cpu/simple/timing.hh 94901e131a7f src/cpu/simple/timing.cc 94901e131a7f src/cpu/testers/directedtest/RubyDirectedTester.hh 94901e131a7f src/cpu/testers/memtest/memtest.hh 94901e131a7f src/cpu/testers/memtest/memtest.cc 94901e131a7f src/cpu/testers/networktest/networktest.hh 94901e131a7f src/cpu/testers/networktest/networktest.cc 94901e131a7f src/cpu/testers/rubytest/RubyTester.hh 94901e131a7f src/cpu/testers/traffic_gen/traffic_gen.hh 94901e131a7f src/cpu/testers/traffic_gen/traffic_gen.cc 94901e131a7f src/dev/dma_device.hh 94901e131a7f src/dev/dma_device.cc 94901e131a7f src/mem/addr_mapper.hh 94901e131a7f src/mem/addr_mapper.cc 94901e131a7f src/mem/bridge.hh 94901e131a7f src/mem/bridge.cc 94901e131a7f src/mem/cache/base.hh 94901e131a7f src/mem/cache/base.cc 94901e131a7f src/mem/cache/cache.hh 94901e131a7f src/mem/cache/cache_impl.hh 94901e131a7f src/mem/coherent_xbar.hh 94901e131a7f src/mem/coherent_xbar.cc 94901e131a7f src/mem/comm_monitor.hh 94901e131a7f src/mem/comm_monitor.cc 94901e131a7f src/mem/dram_ctrl.hh 94901e131a7f src/mem/dram_ctrl.cc 94901e131a7f src/mem/dramsim2.hh 94901e131a7f src/mem/dramsim2.cc 94901e131a7f src/mem/external_slave.cc 94901e131a7f src/mem/mem_checker_monitor.hh 94901e131a7f src/mem/mem_checker_monitor.cc 94901e131a7f src/mem/mport.hh 94901e131a7f src/mem/noncoherent_xbar.hh 94901e131a7f src/mem/noncoherent_xbar.cc 94901e131a7f src/mem/packet_queue.hh 94901e131a7f src/mem/packet_queue.cc 94901e131a7f src/mem/port.hh 94901e131a7f src/mem/port.cc 94901e131a7f src/mem/qport.hh 94901e131a7f src/mem/ruby/slicc_interface/AbstractController.hh 94901e131a7f src/mem/ruby/slicc_interface/AbstractController.cc 94901e131a7f src/mem/ruby/structures/RubyMemoryControl.hh 94901e131a7f src/mem/ruby/system/DMASequencer.hh 94901e131a7f src/mem/ruby/system/DMASequencer.cc 94901e131a7f src/mem/ruby/system/RubyPort.hh 94901e131a7f src/mem/ruby/system/RubyPort.cc 94901e131a7f src/mem/simple_mem.hh 94901e131a7f src/mem/simple_mem.cc 94901e131a7f src/mem/tport.hh 94901e131a7f src/mem/tport.cc 94901e131a7f src/mem/xbar.hh 94901e131a7f src/mem/xbar.cc 94901e131a7f src/sim/system.hh 94901e131a7f Diff: http://reviews.gem5.org/r/2646/diff/ Testing --- Thanks, Andreas Hansson ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2646: mem: Split port retry for all different packet classes
On Feb. 7, 2015, 6:14 p.m., Steve Reinhardt wrote: Awesome! This lack of separate channels for requests responses has bugged me for years, I'm really glad to see it addressed. One minor suggestion: to me, terms like recvRetryReq and recvRetryResp seem backward, since 'retry' is the noun and 'req'/'resp' are adjectives. So in isolation, recvReqRetry makes more sense, because I'm receiving a retry that's specific to requests (i.e., a request-flavored retry), not receiving a request for a retry (as recvRetryReq implies). On the other hand, I can see how there is some parallelism between sendTimingReq and recvRetryReq. But to me the readability in context trumps the parallelism in the abstract (which you only see in places like the commit message where the function names are juxtaposed). Just to overcome the effort hurdle, I think this issue can be fixed with: perl -pi -e 's/([Rr])(etry)R(eq|esp)/$1$3R$2/' Steve Reinhardt wrote: might want to tack a 'g' onto that substitution just in case Makes sense. We might want to drop Timing all together, and simply have sendReq, sendResp, recvSnoopReq etc. I'll bump the patch with the suggested changes. - Andreas --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2646/#review5867 --- On Feb. 7, 2015, 5:24 p.m., Andreas Hansson wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2646/ --- (Updated Feb. 7, 2015, 5:24 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10707:ec8bd9a8e1e2 --- mem: Split port retry for all different packet classes This patch fixes a long-standing isue with the port flow control. Before this patch the retry mechanism was shared between all different packet classes. As a result, a snoop response could get stuck behind a request waiting for a retry, even if the send/recv functions were split. This caused message-dependent deadlocks in stress-test scenarios. The patch splits the retry into one per packet (message) class. Thus, sendTimingReq has a corresponding recvRetryReq, sendTimingResp has recvRetryResp etc. Most of the changes to the code involve simply clarifying what type of request a specific object was accepting. The biggest change in functionality is in the cache downstream packet queue, facing the memory. This queue was shared by requests and snoop responses, and it is now split into two queues, each with their own flow control, but the same physical MasterPort. These changes fixes the previously seen deadlocks. Diffs - src/cpu/minor/fetch1.hh 94901e131a7f src/cpu/kvm/base.hh 94901e131a7f src/arch/x86/pagetable_walker.hh 94901e131a7f src/arch/x86/pagetable_walker.cc 94901e131a7f src/cpu/minor/fetch1.cc 94901e131a7f src/cpu/minor/lsq.hh 94901e131a7f src/cpu/minor/lsq.cc 94901e131a7f src/cpu/o3/cpu.hh 94901e131a7f src/cpu/o3/cpu.cc 94901e131a7f src/cpu/o3/fetch.hh 94901e131a7f src/cpu/o3/fetch_impl.hh 94901e131a7f src/cpu/o3/lsq.hh 94901e131a7f src/cpu/o3/lsq_impl.hh 94901e131a7f src/cpu/simple/atomic.hh 94901e131a7f src/cpu/simple/timing.hh 94901e131a7f src/cpu/simple/timing.cc 94901e131a7f src/cpu/testers/directedtest/RubyDirectedTester.hh 94901e131a7f src/cpu/testers/memtest/memtest.hh 94901e131a7f src/cpu/testers/memtest/memtest.cc 94901e131a7f src/cpu/testers/networktest/networktest.hh 94901e131a7f src/cpu/testers/networktest/networktest.cc 94901e131a7f src/cpu/testers/rubytest/RubyTester.hh 94901e131a7f src/cpu/testers/traffic_gen/traffic_gen.hh 94901e131a7f src/cpu/testers/traffic_gen/traffic_gen.cc 94901e131a7f src/dev/dma_device.hh 94901e131a7f src/dev/dma_device.cc 94901e131a7f src/mem/addr_mapper.hh 94901e131a7f src/mem/addr_mapper.cc 94901e131a7f src/mem/bridge.hh 94901e131a7f src/mem/bridge.cc 94901e131a7f src/mem/cache/base.hh 94901e131a7f src/mem/cache/base.cc 94901e131a7f src/mem/cache/cache.hh 94901e131a7f src/mem/cache/cache_impl.hh 94901e131a7f src/mem/coherent_xbar.hh 94901e131a7f src/mem/coherent_xbar.cc 94901e131a7f src/mem/comm_monitor.hh 94901e131a7f src/mem/comm_monitor.cc 94901e131a7f src/mem/dram_ctrl.hh 94901e131a7f src/mem/dram_ctrl.cc 94901e131a7f src/mem/dramsim2.hh 94901e131a7f src/mem/dramsim2.cc 94901e131a7f src/mem/external_slave.cc 94901e131a7f src/mem/mem_checker_monitor.hh 94901e131a7f src/mem/mem_checker_monitor.cc 94901e131a7f src/mem/mport.hh 94901e131a7f src/mem/noncoherent_xbar.hh 94901e131a7f src/mem/noncoherent_xbar.cc
[gem5-dev] Review Request 2650: cache: add local var in recvTimingResp()
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2650/ --- Review request for Default. Repository: gem5 Description --- Changeset 10685:5be4344f16fe --- cache: add local var in recvTimingResp() The main loop in recvTimingResp() uses target-pkt all over the place. Create a local tgt_pkt to help keep lines under the line length limit. Diffs - src/mem/cache/cache_impl.hh 94901e131a7f2452b3cf2b2740be866b4a6f404d Diff: http://reviews.gem5.org/r/2650/diff/ Testing --- Thanks, Steve Reinhardt ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 2651: cache: remove redundant test in recvTimingResp()
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2651/ --- Review request for Default. Repository: gem5 Description --- Changeset 10686:7fc1f2b733cf --- cache: remove redundant test in recvTimingResp() For some reason we were checking mshr-hasTargets() even though we had already called mshr-getTarget() unconditionally earlier in the same function (which asserts if there are no targets). Get rid of this useless check, and while we're at it get rid of the redundant call to mshr-getTarget(), since we still have the value saved in a local var. Diffs - src/mem/cache/cache_impl.hh 94901e131a7f2452b3cf2b2740be866b4a6f404d Diff: http://reviews.gem5.org/r/2651/diff/ Testing --- Thanks, Steve Reinhardt ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Cron m5test@zizzer /z/m5/regression/do-regression --scratch all
* build/ALPHA/tests/opt/quick/se/30.eio-mp/alpha/eio/simple-timing-mp passed. * build/ALPHA/tests/opt/quick/se/20.eio-short/alpha/eio/simple-timing passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/minor-timing passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby passed. * build/ALPHA/tests/opt/long/se/50.vortex/alpha/tru64/simple-atomic passed. * build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/o3-timing passed. * build/ALPHA/tests/opt/long/se/70.twolf/alpha/tru64/simple-atomic passed. * build/ALPHA/tests/opt/quick/se/20.eio-short/alpha/eio/simple-atomic passed. * build/ALPHA/tests/opt/quick/se/01.hello-2T-smt/alpha/linux/o3-timing passed. * build/ALPHA/tests/opt/long/se/30.eon/alpha/tru64/simple-atomic passed. * build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-minor passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/minor-timing passed. * build/ALPHA/tests/opt/long/se/70.twolf/alpha/tru64/minor-timing passed. * build/ALPHA/tests/opt/long/se/50.vortex/alpha/tru64/o3-timing passed. * build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-o3-dual passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing passed. * build/ALPHA/tests/opt/quick/fs/80.netperf-stream/alpha/linux/twosys-tsunami-simple-atomic passed. * build/ALPHA/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby passed. * build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing passed. * build/ALPHA/tests/opt/long/se/40.perlbmk/alpha/tru64/simple-atomic passed. * build/ALPHA/tests/opt/long/se/50.vortex/alpha/tru64/simple-timing passed. * build/ALPHA/tests/opt/long/se/60.bzip2/alpha/tru64/simple-atomic passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-atomic passed. * build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-switcheroo-full passed. * build/ALPHA/tests/opt/long/se/70.twolf/alpha/tru64/simple-timing passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/o3-timing passed. * build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing-dual passed. * build/ALPHA/tests/opt/long/se/50.vortex/alpha/tru64/minor-timing passed. * build/ALPHA/tests/opt/long/se/30.eon/alpha/tru64/minor-timing passed. * build/ALPHA/tests/opt/long/se/40.perlbmk/alpha/tru64/simple-timing passed. * build/ALPHA/tests/opt/long/se/20.parser/alpha/tru64/minor-timing passed. * build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-atomic passed. * build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual passed. * build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-o3 passed. * build/ALPHA/tests/opt/long/se/30.eon/alpha/tru64/o3-timing passed. * build/ALPHA/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby passed. * build/ALPHA/tests/opt/long/se/30.eon/alpha/tru64/simple-timing passed. * build/ALPHA/tests/opt/quick/se/30.eio-mp/alpha/eio/simple-atomic-mp passed. * build/ALPHA/tests/opt/long/se/70.twolf/alpha/tru64/o3-timing passed. * build/ALPHA/tests/opt/long/se/60.bzip2/alpha/tru64/simple-timing passed. * build/ALPHA_MOESI_hammer/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer passed. * build/ALPHA_MOESI_hammer/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer passed. * build/ALPHA_MOESI_hammer/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer passed. * build/ALPHA_MOESI_hammer/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer passed. * build/ALPHA/tests/opt/long/se/40.perlbmk/alpha/tru64/minor-timing passed. * build/ALPHA/tests/opt/long/se/40.perlbmk/alpha/tru64/o3-timing passed. * build/ALPHA_MESI_Two_Level/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MESI_Two_Level passed. * build/ALPHA_MESI_Two_Level/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MESI_Two_Level passed. * build/ALPHA_MESI_Two_Level/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MESI_Two_Level passed. * build/ALPHA_MESI_Two_Level/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby-MESI_Two_Level passed. * build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_directory passed. * build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory passed. * build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory
