On 09/07, Oleg Nesterov wrote: > > Oh, I disagree. But I guess I can't convince you/Eric/Linus, so I have > to shut up. > > > Damn. But I can't relax ;) Al, Linus, could you comment the patch below? > > Not for inclusion, lacks the changelog/testing, fput() can be simplified. > But as you can see it is simple. With this patch task_work_add(____fput) > will be called only once by (say) do_exit() path. ->fput_list does not > need any serialization / atomic ops / etc. Probably we also need to move > cond_resched() from task_work_run() to ____fput() after this patch. > > Again, it is not that I think this actually makes sense, but since you > dislike these 275ms... > > What do you think?
Yes, task_struct->fput_list is ugly. We can avoid it, but then we need another ->next pointer in struct file. Perhaps we can reuse ->f_version? This way the change looks really simple and not too bad to me. Although I am not sure you will agree. Oleg. --- diff --git a/fs/file_table.c b/fs/file_table.c index 294174d..c34b666 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -241,7 +241,15 @@ static void delayed_fput(struct work_struct *unused) static void ____fput(struct callback_head *work) { - __fput(container_of(work, struct file, f_u.fu_rcuhead)); + struct file *file = container_of(work, struct file, f_u.fu_rcuhead); + struct file *next; + + do { + next = file->f_next_put; + __fput(file); + file = next; + + } while (file); } /* @@ -267,9 +275,21 @@ void fput(struct file *file) struct task_struct *task = current; if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) { + struct callback_head *work = READ_ONCE(task->task_works); + struct file *prev; + + if (work && work->func == ____fput) { + prev = container_of(work, struct file, f_u.fu_rcuhead); + file->f_next_put = prev->f_next_put; + prev->f_next_put = file; + return; + } + init_task_work(&file->f_u.fu_rcuhead, ____fput); - if (!task_work_add(task, &file->f_u.fu_rcuhead, true)) + if (!task_work_add(task, &file->f_u.fu_rcuhead, true)) { + file->f_next_put = NULL; return; + } /* * After this task has run exit_task_work(), * task_work_add() will fail. Fall through to delayed diff --git a/include/linux/fs.h b/include/linux/fs.h index 0774487..9381527 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -849,7 +849,10 @@ struct file { const struct cred *f_cred; struct file_ra_state f_ra; - u64 f_version; + union { + u64 f_version; + struct file *f_next_put; + }; #ifdef CONFIG_SECURITY void *f_security; #endif -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/