Re: [patch] Minor PAGG attach/detach semantic change for 2.6.11

Erik Jacobson <[email protected]>
Newsgroups gmane.linux.process-aggregates
Message-ID <[email protected]>
Ok, I think I understand now.  You're saying since the process never
really was running, it doesn't make sense to run __pnotify_exit.

In __pnotify_fork, I think I agree with you.  If there was a failure 
forking, the kernel module wouldn't expect to be notified for exit.

Also, if the fork callout indicated that the process should fail as a
fork failure, I agree, the kernel module wouldn't expect the the
exit callout.

However, in fork.c's copy_process function, I'm not sure I'm comfortable
changing the pnotify_exit call there to pnotify_unsubscribe.  I don't want
to cause any sort of memory leak where structures a kernel module allocated
are not freed.  

However, we know if our own pnotify_fork call failed, that we don't have
anything to cleanup.  So I made that jump past the pnotify_exit in the
cleanup section.  But if anything after pnotify_fork fails, pnotify_exit
will be called.  This seems pretty normal for copy_process.

Does that sound ok?

I'll post an updated patch shortly with this and some other feedback.


> My suggestion is that the unsubscribe exit callback should only be
> done for clients who successfully subscribed. For example, during a
> fork:
> 
> 1. Client A subscribes with its subscriber->events->fork() being
> successful.
>    
> 2. Client B subscribes with its subscriber->events->fork() being
> successful.
> 
> 3. Client C fails the fork and its subscriber->events->fork() returns
> a failure.  Client C has its subscription removed from the pnotify
> list but its subscriber->events->exit() callback should not be called.
> 
> 4. Client A & B are unsubscribed and their subscriber->events->exit()
> callbacks are invoked. 
> 
> 
> Right now, AFAICS, client C would have its subscriber->events->exit()
> callback invoked in spite of a failure.  IMHO the exit() callback
> should only ever be called for a client for whom its fork() callback
> succeeded.  What do you think?
> 
> > 
> > See the RCU test version of pnotify in the download site under
> > pnotify-test.  My attempts at posting that patch to the list seem to be
> > eaten by the list server right now.  When that's fixed, I'll start posting
> > stuff here.
> > 
> > Erik
> 
> Thanks,
> -- 
> 		Kingsley
--
Erik Jacobson - Linux System Software - Silicon Graphics - Eagan, Minnesota
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.