> On April 4, 2012, 9:23 p.m., Brad Beckmann wrote:
> > Hi Andreas,

Sorry it took me a couple weeks to look this patch over.  It is an impressive 
amount of work.  My biggest concern is it appears that this patch will break 
all existing checkpoints, correct?  Your comment states that a later patch will 
enable checkpoint migration between different NUMA layouts, but will it also 
support checkpoints created prior to this change?  Can you delay checking this 
patch in until that later patch with checkpoint support is ready to be checked 
in?

Thanks
> 
> Andreas Hansson wrote:
>     Hi Brad,
>     
>     Thanks :)
>     
>     The checkpointing will continue to work just as usual with all your 
> existing systems, as they only use one physmem, and that is not going to 
> cause you any problems. At the moment, the serialize/unserialize is still 
> part of each individual memory, so as long as they are not modified 
> before/after the checkpoint it is "business as usual" even with multiple 
> memories. Therefore I'd prefer to get this patch out as it is.
>     
>     Later on, if we want to think about potential features to add, one would 
> be to change the number and type of memories for different distributed memory 
> layouts. That would require moving some intelligence into the 
> "PhysicalMemory" wrapper now as it would have to "distribute" the state to 
> the appropriate memories. This is an added function that should be a separate 
> patch and I will try and add support for this on a "need to have" basis as 
> there is currently a lot in the pipeline. I don't see it as critical to the 
> current patch as it maintains current functionality.
>     
>     Similarly, for the striping, I'm currently looking at how to improve the 
> support for this in gem5. This will also be a separate patch and added later.
>     
>     I hope that all makes sense and that we can commit this as it is.
> 
> Brad Beckmann wrote:
>     I see, so despite replacing the PhysicalMemory instantiations with the 
> SimpleMemory instantiations, everything will still work because the config 
> name of SimpleMemory is still physmem.  Ok, if you've already checked that 
> existing checkpoints work with this change (and I now suspect they do) then 
> go ahead and check this in.
>     
>     Btw, in case you're not aware, some Ruby protocols support numa stripping 
> right now via the numa_high_bit option.

The checkpointng remains unaffected for now, and can be extended later, just as 
the address mapping.

If you're fine with this then I assume I can ship it?


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1113/#review2431
-----------------------------------------------------------


On April 2, 2012, 2:34 p.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1113/
> -----------------------------------------------------------
> 
> (Updated April 2, 2012, 2:34 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> MEM: Enable multiple distributed generalized memories
> 
> This patch removes the assumption on having on single instance of
> PhysicalMemory, and enables a distributed memory where the individual
> memories in the system are each responsible for a single contiguous
> address range.
> 
> All memories inherit from an AbstractMemory that encompasses the basic
> behaviuor of a random access memory, and provides untimed access
> methods. What was previously called PhysicalMemory is now
> SimpleMemory, and a subclass of AbstractMemory. All future types of
> memory controllers should inherit from AbstractMemory.
> 
> To enable e.g. the atomic CPU and RubyPort to access the now
> distributed memory, the system has a wrapper class, called
> PhysicalMemory that is aware of all the memories in the system and
> their associated address ranges. This class thus acts as an
> infinitely-fast bus and performs address decoding for these "shortcut"
> accesses. Each memory can specify that it should not be part of the
> global address map (used e.g. by the functional memories by some
> testers). Moreover, each memory can be configured to be reported to
> the OS configuration table, useful for populating ATAG structures, and
> any potential ACPI tables.
> 
> Checkpointing support currently assumes that all memories have the
> same size and organisation when creating and resuming from the
> checkpoint. A future patch will enable a more flexible
> re-organisation.
> 
> 
> Diffs
> -----
> 
>   configs/example/ruby_network_test.py 97f06a79b6f5 
>   configs/example/ruby_random_test.py 97f06a79b6f5 
>   configs/example/se.py 97f06a79b6f5 
>   configs/ruby/MESI_CMP_directory.py 97f06a79b6f5 
>   configs/ruby/MI_example.py 97f06a79b6f5 
>   configs/example/ruby_mem_test.py 97f06a79b6f5 
>   configs/example/ruby_direct_test.py 97f06a79b6f5 
>   configs/example/memtest.py 97f06a79b6f5 
>   configs/common/FSConfig.py 97f06a79b6f5 
>   configs/ruby/MOESI_CMP_directory.py 97f06a79b6f5 
>   configs/ruby/MOESI_CMP_token.py 97f06a79b6f5 
>   configs/ruby/MOESI_hammer.py 97f06a79b6f5 
>   configs/ruby/Network_test.py 97f06a79b6f5 
>   configs/ruby/Ruby.py 97f06a79b6f5 
>   configs/splash2/cluster.py 97f06a79b6f5 
>   configs/splash2/run.py 97f06a79b6f5 
>   src/arch/alpha/remote_gdb.hh 97f06a79b6f5 
>   src/arch/alpha/remote_gdb.cc 97f06a79b6f5 
>   src/arch/arm/ArmSystem.py 97f06a79b6f5 
>   src/arch/arm/linux/system.cc 97f06a79b6f5 
>   src/arch/arm/system.cc 97f06a79b6f5 
>   src/arch/sparc/SparcSystem.py 97f06a79b6f5 
>   src/base/remote_gdb.hh 97f06a79b6f5 
>   src/base/remote_gdb.cc 97f06a79b6f5 
>   src/cpu/checker/thread_context.hh 97f06a79b6f5 
>   src/cpu/inorder/thread_context.hh 97f06a79b6f5 
>   src/cpu/o3/fetch_impl.hh 97f06a79b6f5 
>   src/cpu/simple/atomic.hh 97f06a79b6f5 
>   src/cpu/simple/atomic.cc 97f06a79b6f5 
>   src/dev/alpha/backdoor.cc 97f06a79b6f5 
>   src/dev/arm/RealView.py 97f06a79b6f5 
>   src/mem/AbstractMemory.py PRE-CREATION 
>   src/mem/PhysicalMemory.py 97f06a79b6f5 
>   src/mem/SConscript 97f06a79b6f5 
>   src/mem/SimpleMemory.py PRE-CREATION 
>   src/mem/abstract_mem.hh PRE-CREATION 
>   src/mem/abstract_mem.cc PRE-CREATION 
>   src/mem/physical.hh 97f06a79b6f5 
>   src/mem/physical.cc 97f06a79b6f5 
>   src/mem/ruby/system/RubyPort.cc 97f06a79b6f5 
>   src/mem/simple_mem.hh PRE-CREATION 
>   src/mem/simple_mem.cc PRE-CREATION 
>   src/sim/System.py 97f06a79b6f5 
>   src/sim/system.hh 97f06a79b6f5 
>   src/sim/system.cc 97f06a79b6f5 
>   tests/configs/inorder-timing.py 97f06a79b6f5 
>   tests/configs/memtest-ruby.py 97f06a79b6f5 
>   tests/configs/memtest.py 97f06a79b6f5 
>   tests/configs/o3-timing-checker.py 97f06a79b6f5 
>   tests/configs/o3-timing-mp.py 97f06a79b6f5 
>   tests/configs/o3-timing.py 97f06a79b6f5 
>   tests/configs/rubytest-ruby.py 97f06a79b6f5 
>   tests/configs/simple-atomic-dummychecker.py 97f06a79b6f5 
>   tests/configs/simple-atomic-mp.py 97f06a79b6f5 
>   tests/configs/simple-atomic.py 97f06a79b6f5 
>   tests/configs/simple-timing-mp-ruby.py 97f06a79b6f5 
>   tests/configs/simple-timing-mp.py 97f06a79b6f5 
>   tests/configs/simple-timing-ruby.py 97f06a79b6f5 
>   tests/configs/simple-timing.py 97f06a79b6f5 
> 
> Diff: http://reviews.gem5.org/r/1113/diff/
> 
> 
> Testing
> -------
> 
> util/regress all passing (disregarding t1000 and eio)
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to