lkml.org 
[lkml]   [2014]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] fsnotify: don't call mutex_lock from TASK_INTERRUPTIBLE context
On Sat,  1 Nov 2014 23:51:38 -0400 Sasha Levin <sasha.levin@oracle.com> wrote:

> Sleeping functions should only be called from TASK_RUNNING. The following
> code in fanotify_read():
>
> prepare_to_wait(&group->notification_waitq, &wait, TASK_INTERRUPTIBLE);
>
> mutex_lock(&group->notification_mutex);
>
> would call it under TASK_INTERRUPTIBLE, and trigger a warning:
>
> [12326.092094] WARNING: CPU: 27 PID: 30207 at kernel/sched/core.c:7305 __might_sleep+0xd2/0x110()
> [12326.092878] do not call blocking ops when !TASK_RUNNING; state=1 set at prepare_to_wait (./arch/x86/include/asm/current.h:14 kernel/sched/wait.c:179)
> [12326.093938] Modules linked in:
>
> ...
>

It's a fairly minor problem - if mutex_lock() hits contention we get
flipped into TASK_RUNNING and the schedule() immediately returns and we
take another trip around the loop.

fanotify_read() also calls copy_event_to_user()->copy_to_user() in
TASK_INTERRUPTIBLE state. That's a bug and this is why the first thing
handle_mm_fault() does is to set TASK_RUNNING.

> Instead of trying to fix fanotify_read() I've converted
> notification_mutex into a spinlock. I didn't see a reason why it
> should be a mutex nor anything complained when I ran the same tests
> again.

This could be a latency problem - those lists can get very long.

I wonder if we can zap the prepare_to_wait()/finish_wait() and use
something like

wait_event_interruptible(notification_waitq, foo(group, count));

int foo(struct fsnotify_group *group, size_t count)
{
int ret;

mutex_lock(&group->notification_mutex);
ret = get_one_event(group, count);
mutex_unlock(&group->notification_mutex);
}



\
 
 \ /
  Last update: 2014-11-04 23:21    [W:2.014 / U:0.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site