Re: 2.6.7 pagg patch available

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                                   [email protected]

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce