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

[Erik Jacobson <erikj@xxxxxxx>]

Hi Again.  Unless I'm missing something (and that sure has happened in
this conversation so it's probably the case :) --

I wrote copy_process to skip the fork_exit on purpose.  

1. If there isn't enough memory for the pnotify_subscribe, we
   return -ENOMEM, which will be a fork failure.  In this case,
   the kernel module wasn't notified via the callout.  So it wouldn't
   make sense to pnotify_exit for this process in copy_process.

2. If the kernel module itself "vetoed" the subscriber, well, the kernel
   module is the one who directed the action and should know a 
   pnotify_exit will never be called for that process as that was the
   reason for the veto anyway.  So we just unsubscribe it.

3. If the kernel module returned a failure, the kernel module itself knows
   the process is going to fail in fork.  It wouldn't have allocated memory
   for data it knew would get tossed in just a moment anyway.  It seems wrong
   to have fork_exit called when the kernel module is who directed the 
   failure.  So we just unsubscribe it.

4. If the fork callout from the kernel module is a success and 
   __pnotify_fork is a success, we hop back in to copy_process all happy.
   But if, after the pnotify_fork, the fork fails...  In that case, we
   call pnotify_exit because there would be no way for the kernel module to
   know about the fork failing after pnotify_fork is completed.  

I think this covers all the cases,  If I'm wrong, please try again.  If that's
the case, I'm sorry it's taking me so long to understand :)

So the skipping of pnotify_exit is on purpose in copy_process.  We only
call pnotify_exit in the case that the kernel module doesn't know about
the failure for some reason because the failure happened after pnotify_fork
completed.

+__pnotify_fork(struct task_struct *to_task, struct task_struct *from_task)
+{
+  struct pnotify_subscriber *from_subscriber;
+  int ret;
+
+  /* lock the parents subscriber list we are copying from */
+  down_read(&from_task->pnotify_subscriber_list_sem);
+
+  list_for_each_entry(from_subscriber,
+            &from_task->pnotify_subscriber_list, entry) {
+     struct pnotify_subscriber *to_subscriber = NULL;
+
+     to_subscriber = pnotify_subscribe(to_task,
+                   from_subscriber->events);
+     if (!to_subscriber) {
+        /* Failed to get memory.
+         * we don't force __pnotify_exit to run here because
+         * the child is in-consturction and not running yet.
+         * We don't need a write lock on the subscriber
+         * list because the child is in construction.
+         */
+        pnotify_unsubscribe(to_task);
+        up_read(&from_task->pnotify_subscriber_list_sem);
+        return -ENOMEM;
+     }
+     ret = to_subscriber->events->fork(to_task, to_subscriber,
+                   from_subscriber->data);
+
+     if (ret < 0) {
+        /* Propagates to copy_process as a fork failure.
+         * Since the child is in consturction, we don't
+         * need a write lock on the subscriber list.
+         * __pnotify_exit isn't run because the child
+         * never got running, exit doesn't make sense.
+         */
+        pnotify_unsubscribe(to_task);
+        up_read(&from_task->pnotify_subscriber_list_sem);
+        return ret; /* Fork failure */
+     }
+     else if (ret > 0) {
+        /* Success, but fork function pointer in the
+         * pnotify_events structure doesn't want the kernel
+         * module subscribed.  This is an in-construction
+         * child so we don't need to write lock */
+        pnotify_unsubscribe(to_subscriber);
+     }
+  }
+
+  /* unlock parent's subscriber list */
+  up_read(&from_task->pnotify_subscriber_list_sem);
+
+  return 0; /* success */
+}