:Hi,
:
:Here is a second pass at this; combine the last three patches in git to get
:a total picture:
:
:http://gitweb.dragonflybsd.org/~vsrinivas/dragonfly_zerothread.git
:
:Any comments would be nice; in particular, the vm_pagezero() function turned
:out more complex than I'd hoped, any help restructuring it would rock
:
:Thanks, world!
I added the export file to the repo so it can be exported. People should
use the git:// path instead:
git://leaf.dragonflybsd.org/~vsrinivas/dragonfly_zerothread.git
Also when someone gets the chance on IRC give Venkatesh a run-down
on how to use branch names.
--
Ok, here are my comments:
bzeront addition:
Could probably be moved to sys/cpu to reduce code duplication (just
have one for x86_64 and one for i386 instead of one for each
platform).
This was my bad, on IRC I suggested platform/ but now that I see
the code it is clear it should be in cpu/
cnt_prezero sysctl
should probably be moved to vm_zeroidle.c
TDPRI_IDLE_WORK
good.
vm_page_free_fromq_fast() and vm_page_free_toq_fast()
Hmm. I would say maybe have vm_page_free_fromq_fast() busy the
VM page, leave it busy through the entire zeroing process, and
then get rid of vm_page_free_toq_fast() and use the normal
page freeing function. Set PG_ZERO in vm_zeroidle.c after
successfully zeroing the page.
vm_zeroidle.c
Adjust syntax for comments a bit to match the rest of the codebase.
e.g.:
/* comment
* comment
*/
to
/*
* comment
* comment
*/
NTSTORE_AVAIL conditionalization needs to be cleaned up. Make a
cpu_*() call in the platform code to return whether the feature is
available or not and set a local variable outside the zeroing loop.
The main loop is a bit confusing with all the gotos. I recommend
a state machine switch.
* Change SUSPENDED_* to an enumeration.
* switch() on the state.
* fall through to the tsleep or place the tsleep before the break in
each case statement.
* Your tsleep() is set to sleep for 5 minutes? I think you might
want to change that to, say, once a second (hz instead of hz * 300).
In fact, you don't really need to have a call-in for the wakeup.
The loop can just run off an internal sleep/timer and calculate
a burst of work to do.
* Add an additional state for the normal 'waiting for some work to
do' case which can then set the state to the page acquisition state
and fall through. Also make the state names more informative.
like this:
(Ignore my indentations here, I'm just throwing this down quickly)
for (;;) {
switch(state) {
case STATE_IDLE:
tsleep(&zero_state, 0, "idle", hz);
npages = (calculate number of pages to try to zero)
if (npages)
state = STATE_GET_PAGE;
break;
case STATE_GET_PAGE:
if (try_mplock() == 0) {
tsleep(&zero_state, 0, "idle", 1);
/* try again soon */
break;
}
if (--npages == 0) {
state = STATE_IDLE;
} else {
m = vm_page_free_fromq_fast();
if (m == NULL) {
state = STATE_IDLE;
} else {
state = STATE_ZERO_PAGE;
buf = .... get the lwbuf
}
}
rel_mplock();
break;
case STATE_ZERO_PAGE:
... loop to zero page ....
... just restart state if we have to break out due
to a resched ...
break;
case STATE_RELEASE_PAGE:
if (try_mplock() == 0)
break;
... release the page, release the lwbuf, etc ...
state = STATE_GET_PAGE;
rel_mplock();
break;
}
lwkt_switch();
}
Something like that.
-Matt
