Messages in this thread Patch in this message |  | | Date | Mon, 15 Apr 2013 17:33:07 +0200 | | From | Oleg Nesterov <> | | Subject | Re: [PATCH 1/1] move exit_task_namespaces() outside of exit_notify() |
| |
On 04/15, Andrey Wagin wrote: > > It looks good for me. I have tested it a bit and don't find any problem. > Oleg, thank you. > > Acked-by: Andrew Vagin <avagin@gmail.com>
Thanks Andrey and Eric.
> > --- x/kernel/exit.c > > +++ x/kernel/exit.c > > @@ -649,7 +649,6 @@ static void exit_notify(struct task_stru > > * jobs, send them a SIGHUP and then a SIGCONT. (POSIX 3.2.2.2) > > */ > > forget_original_parent(tsk); > > - exit_task_namespaces(tsk); > > > > write_lock_irq(&tasklist_lock); > > if (group_dead) > > @@ -795,6 +794,7 @@ void do_exit(long code) > > exit_shm(tsk); > > exit_files(tsk); > > exit_fs(tsk); > > + exit_task_namespaces(tsk); > > exit_task_work(tsk);
I do not see any problems with this patch too... but still I am worried.
Even if fput() can work correctly after exit_task_namespaces(), this limits the usage of task_work_add(). Probably this is fine, but can't we at least discuss another change?
We can change fput() so that it can always work, even after exit_task_work(),
void fput(struct file *file) { if (atomic_long_dec_and_test(&file->f_count)) { struct task_struct *task = current; unsigned long flags;
file_sb_list_del(file); if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) { init_task_work(&file->f_u.fu_rcuhead, ____fput); if (!task_work_add(task, &file->f_u.fu_rcuhead, true)) return; }
spin_lock_irqsave(&delayed_fput_lock, flags); list_add(&file->f_u.fu_list, &delayed_fput_list); schedule_work(&delayed_fput_work); spin_unlock_irqrestore(&delayed_fput_lock, flags); } }
Al, what do you think?
Untested patch below.
Oleg.
--- x/fs/file_table.c +++ x/fs/file_table.c @@ -306,17 +306,19 @@ void fput(struct file *file) { if (atomic_long_dec_and_test(&file->f_count)) { struct task_struct *task = current; + unsigned long flags; + file_sb_list_del(file); - if (unlikely(in_interrupt() || task->flags & PF_KTHREAD)) { - unsigned long flags; - spin_lock_irqsave(&delayed_fput_lock, flags); - list_add(&file->f_u.fu_list, &delayed_fput_list); - schedule_work(&delayed_fput_work); - spin_unlock_irqrestore(&delayed_fput_lock, flags); - return; + if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) { + init_task_work(&file->f_u.fu_rcuhead, ____fput); + if (!task_work_add(task, &file->f_u.fu_rcuhead, true)) + return; } - init_task_work(&file->f_u.fu_rcuhead, ____fput); - task_work_add(task, &file->f_u.fu_rcuhead, true); + + spin_lock_irqsave(&delayed_fput_lock, flags); + list_add(&file->f_u.fu_list, &delayed_fput_list); + schedule_work(&delayed_fput_work); + spin_unlock_irqrestore(&delayed_fput_lock, flags); } }
|  |