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 *file, char __user *buf, group = file->private_data; while (1) { + mutex_lock(&group->notification_mutex); prepare_to_wait(&group->notification_waitq, &wait, TASK_INTERRUPTIBLE); - mutex_lock(&group->notification_mutex); kevent = get_one_event(group, count); mutex_unlock(&group->notification_mutex); pr_debug("%s: group=%p kevent=%p\n", __func__, group, kevent); if (kevent) { + __set_current_state(TASK_RUNNING); ret = PTR_ERR(kevent); if (IS_ERR(kevent)) break;
On 08/04/2014 06:30 PM, Peter Zijlstra wrote: > 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 this by using the new woken_wake_function and wait_woken() stuff, > which registers wakeups in wait and thereby allows shrinking the > task_state::state changes to the actual sleep part. > > Cc: Eric Paris <epa...@parisplace.org> > Cc: John McCutchan <j...@johnmccutchan.com> > Cc: Robert Love <rl...@rlove.org> > Signed-off-by: Peter Zijlstra <pet...@infradead.org> > --- > fs/notify/inotify/inotify_user.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > --- a/fs/notify/inotify/inotify_user.c > +++ b/fs/notify/inotify/inotify_user.c > @@ -227,14 +227,13 @@ static ssize_t inotify_read(struct file > struct fsnotify_event *kevent; > char __user *start; > int ret; > - DEFINE_WAIT(wait); > + DEFINE_WAIT_FUNC(wait, woken_wake_function); > > start = buf; > group = file->private_data; > > + add_wait_queue(&group->notification_waitq, &wait); > while (1) { > - prepare_to_wait(&group->notification_waitq, &wait, > TASK_INTERRUPTIBLE); > - > mutex_lock(&group->notification_mutex); > kevent = get_one_event(group, count); > mutex_unlock(&group->notification_mutex); > @@ -264,10 +263,10 @@ static ssize_t inotify_read(struct file > if (start != buf) > break; > > - schedule(); > + wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT); > } > + remove_wait_queue(&group->notification_waitq, &wait); > > - finish_wait(&group->notification_waitq, &wait); > if (start != buf && ret != -EFAULT) > ret = buf - start; > return ret; > > > -- > 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/ > . > -- 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/