Ok, so the new API is that the system manages physical pages, the process manages virtual pages, the page table glues them together, and there is/are function(s) on the process object which tie it all together? That makes sense. I'll think about how that fits with the whole workload/address space thing I'd like to do at some point and how that extra level of abstraction would fit, but no problems leap out at me.

I'll have to look at the pid thing and remind myself what it's used for. Maybe we can get rid of it too. I don't want to get too tangled up with this other stuff, though, since I'm making good progress on the SE/FS stuff. I want to get that in as soon as it's ready so it doesn't bit rot (it probably will quickly).

Gabe

Quoting Steve Reinhardt <[email protected]>:

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



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

Reply via email to