Erik Jacobson wrote:
Did you include my modifications in this patch?
All I did was resolve the build issues for 2.6.7 and tested to be sure it
still worked.
I sort of have a dilema here. We're going to post pagg and job to LKML
tomorrow and I don't know how to test all the changes.
You had mentioned you didn't think adding the new features would make it
harder to accept. But, on the other hand, what we have now is well
understood and we have test cases for most situations. The pagg code we
have no has been through the ringer once already.
But it's incomplete and doesn't even completely implement its interface.
What do you think? Any other thoughts?
I think that you should at least put the changes that I sent that
implement "per task initialization" during registration and do the
automatic clean up at deregistration.
Without the first of these you have a callback in your interface which
doesn't get used (and that's bad).
Without the second of these writing a "safe" client is much harder than
it needs to be. The current implementation refuses to deregister if the
client still has paggs attached to any tasks and this can lead to the
system getting into an inconsistent state during the unloading of a
client kernel module. My changes use the IMHO better option of
detaching these paggs and calling the clients detach() callback on them
which means that writing a client that loads and unloads safely is a doddle.
The extra callbacks for ruid, rgid and CPU affinity are a different
issue but I think that they will be acceptable to LKML. They have the
advantage of making PAGG more useful and increasing the chances of more
clients being created and made public (and also decreases the chances
someone else will create a competing patch to PAGG because PAGG doesn't
do enough). On the other hand, I don't think that they're urgent and
they can wait until later if you're worried about rocking the boat.
Looking at the NIKM announcement (that I forwarded you earlier) in more
detail it would appear that attach(), detach() and exec() are sufficient
for their purposes (you just have to convince them to use it :-)) so
leaving these changes out is a safe option for now.
Peter
PS I think that I sent my patches in two parts so that you can apply the
essential (or more precisely the ones that I think are essential :-))
ones without getting the others. BTW the "essential" patches only
modify the PAGG files and don't touch any other kernel files so they
should apply to your 2.6.7 version without any trouble.
--
Peter Williams pwil3058@xxxxxxxxxxxxxx
"Learning, n. The kind of ignorance distinguishing the studious."
-- Ambrose Bierce
|