Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Al Viro
On Wed, Aug 22, 2012 at 01:27:21PM +0800, Michael Wang wrote: > And can we make sure that it is safe to sleep(schedule) at this point? > It may need some totally testing to cover all the situation... task_work callback can bloody well block, so yes, it is safe. Hell, we are doing final close

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Michael Wang
Hi, Eric On 08/21/2012 09:05 PM, Eric Dumazet wrote: > From: Eric Dumazet > > It seems commit 4a9d4b02 (switch fput to task_work_add) reintroduced > the problem addressed in commit 944be0b2 (close_files(): add scheduling > point) > > If a server process with a lot of files (say 2 million tcp

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Mimi Zohar
On Tue, 2012-08-21 at 23:32 +0200, Eric Dumazet wrote: > On Tue, 2012-08-21 at 16:37 -0400, Mimi Zohar wrote: > > > We're here, because fput() called schedule_work() to delay the last > > fput(). The execution needs to take place before the syscall returns to > > userspace. Need to read

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Eric Dumazet
On Tue, 2012-08-21 at 16:37 -0400, Mimi Zohar wrote: > We're here, because fput() called schedule_work() to delay the last > fput(). The execution needs to take place before the syscall returns to > userspace. Need to read __schedule()... Do you know if cond_resched() > can guarantee that it

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Mimi Zohar
On Tue, 2012-08-21 at 15:05 +0200, Eric Dumazet wrote: > From: Eric Dumazet > > It seems commit 4a9d4b02 (switch fput to task_work_add) reintroduced > the problem addressed in commit 944be0b2 (close_files(): add scheduling > point) > > If a server process with a lot of files (say 2 million tcp

[PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Eric Dumazet
From: Eric Dumazet It seems commit 4a9d4b02 (switch fput to task_work_add) reintroduced the problem addressed in commit 944be0b2 (close_files(): add scheduling point) If a server process with a lot of files (say 2 million tcp sockets) is killed, we can spend a lot of time in task_work_run() and

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Eric Dumazet
On Tue, 2012-08-21 at 16:37 -0400, Mimi Zohar wrote: We're here, because fput() called schedule_work() to delay the last fput(). The execution needs to take place before the syscall returns to userspace. Need to read __schedule()... Do you know if cond_resched() can guarantee that it will

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Mimi Zohar
On Tue, 2012-08-21 at 23:32 +0200, Eric Dumazet wrote: On Tue, 2012-08-21 at 16:37 -0400, Mimi Zohar wrote: We're here, because fput() called schedule_work() to delay the last fput(). The execution needs to take place before the syscall returns to userspace. Need to read __schedule()...

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Michael Wang
Hi, Eric On 08/21/2012 09:05 PM, Eric Dumazet wrote: From: Eric Dumazet eduma...@google.com It seems commit 4a9d4b02 (switch fput to task_work_add) reintroduced the problem addressed in commit 944be0b2 (close_files(): add scheduling point) If a server process with a lot of files (say 2

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Al Viro
On Wed, Aug 22, 2012 at 01:27:21PM +0800, Michael Wang wrote: And can we make sure that it is safe to sleep(schedule) at this point? It may need some totally testing to cover all the situation... task_work callback can bloody well block, so yes, it is safe. Hell, we are doing final close from

[PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Eric Dumazet
From: Eric Dumazet eduma...@google.com It seems commit 4a9d4b02 (switch fput to task_work_add) reintroduced the problem addressed in commit 944be0b2 (close_files(): add scheduling point) If a server process with a lot of files (say 2 million tcp sockets) is killed, we can spend a lot of time in

Re: [PATCH] task_work: add a scheduling point in task_work_run()

2012-08-21 Thread Mimi Zohar
On Tue, 2012-08-21 at 15:05 +0200, Eric Dumazet wrote: From: Eric Dumazet eduma...@google.com It seems commit 4a9d4b02 (switch fput to task_work_add) reintroduced the problem addressed in commit 944be0b2 (close_files(): add scheduling point) If a server process with a lot of files (say 2