Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-05 Thread Peter Zijlstra
On Tue, Aug 05, 2014 at 10:22:11AM +0800, Lai Jiangshan wrote: > I don't think this one needs nested sleeps. > > diff --git a/fs/notify/inotify/inotify_user.c > b/fs/notify/inotify/inotify_user.c > index cc423a3..1ca5888 100644 > --- a/fs/notify/inotify/inotify_user.c > +++

Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-05 Thread Peter Zijlstra
On Tue, Aug 05, 2014 at 10:22:11AM +0800, Lai Jiangshan wrote: I don't think this one needs nested sleeps. diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index cc423a3..1ca5888 100644 --- a/fs/notify/inotify/inotify_user.c +++

Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-04 Thread Lai Jiangshan
I don't think this one needs nested sleeps. diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index cc423a3..1ca5888 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -233,15 +233,16 @@ static ssize_t inotify_read(struct file

Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-04 Thread Peter Zijlstra
On Mon, Aug 04, 2014 at 09:23:58PM +0200, Oleg Nesterov wrote: > On 08/04, Peter Zijlstra wrote: > > > > while (1) { > > - prepare_to_wait(>notification_waitq, , > > TASK_INTERRUPTIBLE); > > - > > mutex_lock(>notification_mutex); > > So yes, even these 2 lines look

Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-04 Thread Oleg Nesterov
On 08/04, Peter Zijlstra wrote: > > while (1) { > - prepare_to_wait(>notification_waitq, , > TASK_INTERRUPTIBLE); > - > mutex_lock(>notification_mutex); So yes, even these 2 lines look obviously buggy. Even if

[RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-04 Thread Peter Zijlstra
inotify_read is a wait loop with sleeps in. Wait loops rely on task_struct::state and sleeps do too, since that's the only means of actually sleeping. Therefore the nested sleeps destroy the wait loop state and the wait loop breaks the sleep functions that assume TASK_RUNNING (mutex_lock). Fix

[RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-04 Thread Peter Zijlstra
inotify_read is a wait loop with sleeps in. Wait loops rely on task_struct::state and sleeps do too, since that's the only means of actually sleeping. Therefore the nested sleeps destroy the wait loop state and the wait loop breaks the sleep functions that assume TASK_RUNNING (mutex_lock). Fix

Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-04 Thread Oleg Nesterov
On 08/04, Peter Zijlstra wrote: while (1) { - prepare_to_wait(group-notification_waitq, wait, TASK_INTERRUPTIBLE); - mutex_lock(group-notification_mutex); So yes, even these 2 lines look obviously buggy. Even if

Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-04 Thread Peter Zijlstra
On Mon, Aug 04, 2014 at 09:23:58PM +0200, Oleg Nesterov wrote: On 08/04, Peter Zijlstra wrote: while (1) { - prepare_to_wait(group-notification_waitq, wait, TASK_INTERRUPTIBLE); - mutex_lock(group-notification_mutex); So yes, even these 2 lines look

Re: [RFC][PATCH 4/7] inotify: Deal with nested sleeps

2014-08-04 Thread Lai Jiangshan
I don't think this one needs nested sleeps. diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index cc423a3..1ca5888 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -233,15 +233,16 @@ static ssize_t inotify_read(struct file