pagg
[Top] [All Lists]

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

To: kingsley@xxxxxxxxxx
Subject: Re: [patch] Minor PAGG attach/detach semantic change for 2.6.11
From: Erik Jacobson <erikj@xxxxxxx>
Date: Fri, 30 Sep 2005 15:58:58 -0500
Cc: Erik Jacobson <erikj@xxxxxxx>, pagg@xxxxxxxxxxx, tonyt@xxxxxxxxxx
In-reply-to: <20050929223827.GA2737@aurema.com>
References: <20050617014512.GA10285@aurema.com> <20050927201020.GA30433@sgi.com> <20050929051627.GC3404@aurema.com> <20050929151205.GB7395@sgi.com> <20050929223827.GA2737@aurema.com>
Sender: pagg-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.6i
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

<Prev in Thread] Current Thread [Next in Thread>