lkml.org 
[lkml]   [2013]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
Subject[PATCH 0/3] (Was: fput: task_work_add() can fail if the caller has passed exit_task_work())
On 06/14, Andrew Morton wrote:
>
> On Fri, 14 Jun 2013 21:09:47 +0200 Oleg Nesterov <oleg@redhat.com> wrote:
>
> > + 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;
>
> A comment here would be useful, explaining the circumstances under
> which we fall through to the delayed fput.

Thanks!

> This is particularly needed
> because kernel/task_work.c is such undocumented crap.

It seems that you are trying to force me to make the doc patch ;)
OK, I'll try. task_work.c needs a couple of cosmetic cleanups anyway.

> > + 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);
>
> OT: I don't think that schedule_work() needs to be inside the locked
> region. Scalability improvements beckon!

Yeees, I thought about this too.

Performance-wise this can't really help, this case is unlikely. But
I think this change makes this code a bit simpler, so please see 1/3.

2/3 fixes the (theoretical) bug in llist_add() and imho cleanups the
code.

3/3 comes as a separate change because I do not want to argue if
someone dislike the non-inline llist_add(). But once again, we can
make llist_add_batch() inline, and I believe it is never good to
duplicate the code even if it is simple.

Oleg.



\
 
 \ /
  Last update: 2013-06-15 19:41    [W:0.081 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site