On Fri, Oct 21, 2011 at 1:25 PM, Gabriel Michael Black < [email protected]> wrote:
> This is an interesting patch, but I'd like to understand the high level of > what it's doing better. It looks like you're essentially changing who is > responsible for what portion of address space/memory allocation and > management. Could you please break down who (process/page table/system) was > responsible for those parts (phys/virt/mapping)? > It's not changing any responsibility, it's just moving around the interface to eliminate some needless redirection and code dependence. The caller is specifying a set of virtual addresses that it wants mapped to newly allocated physical addresses. The mapping is done by the page table, but the physical addresses are managed by the system. These two objects don't directly know about each other though; the process is the object that ties these two things together, so that seems like the logical place for the interface. Previously the method was on the page table, which had to indirect through the process pointer to get to the system to allocate physical pages. By moving the method to the process, and cleaning up a couple of other minor things in the page table, the page table now is completely independent of the process object, which isn't any immediate benefit but sure seems like a good thing (e.g., if you ever wanted to use the page table independently of a process object). Actually this all came about because I needed a method to add a mapping to the page table without allocating new physical memory pages and was surprised to find that there was none. Once I created that method and factored that code out of allocate(), it became apparent that allocate() didn't really belong with the page table. > Also, I didn't see anywhere the new pid you added is used. It's good to > loosen the coupling between these objects which I think that does, although > I'm curious why the page table needs to know what the pid is at all. Debug > messages? > I didn't add the pid at all, it was already referenced in the code as process->M5_pid. I'm just passing it in explicitly now so that I can eliminate the need for the page table to have a process pointer. See line 83 in page_table.cc. Steve > Gabe > > Quoting Steve Reinhardt <[email protected]>: > > >> ------------------------------**----------------------------- >> >> This is an automatically generated e-mail. To reply, visit: >> http://reviews.m5sim.org/r/**896/ <http://reviews.m5sim.org/r/896/> >> ------------------------------**----------------------------- >> >> >> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and >> Nathan Binkert. >> >> >> Summary >> ------- >> >> >> SE: move page allocation from PageTable to Process >> >> PageTable supported an allocate() call that called back >> through the Process to allocate memory, but did not have >> a method to map addresses without allocating new pages. >> It makes more sense for Process to do the allocation, so >> this method was renamed allocateMem() and moved to Process, >> and uses a new map() call on PageTable. >> >> The remaining uses of the process pointer in PageTable >> were only to get the name and the PID, so by passing these >> in directly in the constructor, we can make PageTable >> completely independent of Process. >> >> >> Diffs >> ----- >> >> src/arch/alpha/process.cc c7fec2cb91cb >> src/arch/arm/linux/process.cc c7fec2cb91cb >> src/arch/arm/process.cc c7fec2cb91cb >> src/arch/mips/process.cc c7fec2cb91cb >> src/arch/power/process.cc c7fec2cb91cb >> src/arch/sparc/process.cc c7fec2cb91cb >> src/arch/x86/process.cc c7fec2cb91cb >> src/kern/tru64/tru64.hh c7fec2cb91cb >> src/mem/page_table.hh c7fec2cb91cb >> src/mem/page_table.cc c7fec2cb91cb >> src/mem/translating_port.cc c7fec2cb91cb >> src/sim/process.hh c7fec2cb91cb >> src/sim/process.cc c7fec2cb91cb >> src/sim/syscall_emul.hh c7fec2cb91cb >> src/sim/syscall_emul.cc c7fec2cb91cb >> src/sim/system.hh c7fec2cb91cb >> src/sim/system.cc c7fec2cb91cb >> >> Diff: >> http://reviews.m5sim.org/r/**896/diff<http://reviews.m5sim.org/r/896/diff> >> >> >> Testing >> ------- >> >> >> Thanks, >> >> Steve >> >> ______________________________**_________________ >> gem5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/**listinfo/gem5-dev<http://m5sim.org/mailman/listinfo/gem5-dev> >> >> > > ______________________________**_________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/**listinfo/gem5-dev<http://m5sim.org/mailman/listinfo/gem5-dev> > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
