Hi Erik,
I've had a chance to look at the issue and have come up with two
patches. This email has the first of two patches. This patch
addresses the first problem described below by using the "thread"
macros during registration and deregistration. It also makes a check
to skip exiting processes.
On Tue, Nov 09, 2004 at 10:00:39AM +1100, Kingsley Cheung wrote:
> Hi Erik,
>
> Some other issues as well. Please correct me if I'm wrong :)
>
> 1) I've noticed that pagg_detach() has been moved from
> __put_task_struct() to do_exit() between the pagg patches for Linux
> 2.6.8. This occurred as described at
> http://oss.sgi.com/archives/pagg/2004-08/msg00002.html
>
> Anyway, I think the move might have raised a problem with the
> registration code. Below is the relevant segment of code I'd like to
> consider:
>
> get_task_struct(p);
> read_unlock(&tasklist_lock);
> down_write(&p->pagg_sem);
> paggp = pagg_get(p, pagg_hook_new->name);
> if (paggp == NULL) {
> paggp = pagg_alloc(p, pagg_hook_new);
> if (paggp != NULL)
> init_result = pagg_hook_new->init(p, paggp);
> else
> init_result = -ENOMEM;
> }
> up_write(&p->pagg_sem);
> read_lock(&tasklist_lock);
> /* Like in remove_client_paggs_from_all_tasks, if the task
> * disappeared on us while we were going through the
> * for_each_process loop, we need to start over with that loop.
> * That's why we have the list_empty here */
> task_exited = list_empty(&p->tasks);
> put_task_struct(p);
>
> Now the tasks on the task list include those that are zombies. This
> occurs during exit_notify(), which sets their state to ZOMBIE but
> leaves them on the task list. Therefore the above code will allocate
> paggs for zombie tasks as well. [Note that task_exited won't trigger
> as the task is still on the task list.]
>
> Now unfortunately, by moving pagg_detach() out of __put_task_struct(),
> when the zombie has its task structure released, the pagg that was
> allocated during registration is no longer detached. Thus we'll could
> have a memory leak once the task structure is dropped and
> detach_pagg() is not invoked.
>
> In fact the leak could happen not just for zombies, but for tasks just
> about to exit. The race occurs in do_exit(), in which the
> detach_pagg() could be called before we allocate the pagg.
>
> Perhaps the quickest solution is to place a detach_pagg() call back
> into __put_task_struct(). Another suggestion might be to check for
> the PF_EXITING flag when allocating the pagg.
Thanks,
--
Kingsley
linux-2.6.9-pagg-threading.patch
Description: Text document
|