Re: deferring __fput()

[Al Viro]

On Sat, Jun 23, 2012 at 10:20:49AM +0100, Al Viro wrote:

> What I have in mind is something like
> 	if (unlikely(in_atomic() || current->flags & PF_KTHREAD))
> 		move file to global list, use schedul_work() to have it dealt with
> 	else
> 		move to per-task list, do task_work_add() if it was empty (i.e.
> 		if we hadn't scheduled its emptying already).
> The latter is completely thread-synchronous, so we shouldn't need any locking
> whatsoever.  The former... I'd probably go for a single list, protected by
> spin_lock_irq(), keeping the possibility to switch to per-CPU lists if we
> find a load that gets serious contention on that list.  In any case, worker will
> start with taking the list contents, emptying the list and then killing the
> suckers off one by one.

BTW, I really wonder why do we need to have that void *data in task_work; we can
always embed the sucker into a bigger struct (if nothing else, task_work +
void *data) and get to it via container_of().  And in quite a few cases we don't
want that data thing at all.  Moreover, the reasons to use hlist_head instead of
a single forward pointer are very thin on the ground:
	* task_work_add() adds to the beginning of the list
	* task_work_cancel() walks the list to find the entry to remove
	* trask_work_run() takes the list out, walks it to the end,
then walks it backwards via pprev.  Could as well reverse the pointers
while walking forward...

Oleg, do you see any reasons why trimming it down to forward pointer + callback
pointer wouldn't work?  Matter of fact, it would become identical to struct rcu_head
after that...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/