>> On Fri 16-02-18 15:14:40, t.vivek@xxxxxxxxxxx wrote: >> From: Vivek Trivedi <t.vivek@xxxxxxxxxxx> >> >> If fanotify userspace response server thread is frozen first, >> it may fail to send response from userspace to kernel space listener. >> In this scenario, fanotify response listener will never get response >> from userepace and fail to suspend. >> >> Use freeze-friendly wait API to handle this issue. >> >> Same problem was reported here: >> https://bbs.archlinux.org/viewtopic.php?id=232270 >> >> Freezing of tasks failed after 20.005 seconds >> (1 tasks refusing to freeze, wq_busy=0) >> >> Backtrace: >> [<c0582f80>] (__schedule) from [<c05835d0>] (schedule+0x4c/0xa4) >> [<c0583584>] (schedule) from [<c01cb648>] (fanotify_handle_event+0x1c8/0x218) >> [<c01cb480>] (fanotify_handle_event) from [<c01c8238>] (fsnotify+0x17c/0x38c) >> [<c01c80bc>] (fsnotify) from [<c02676dc>] (security_file_open+0x88/0x8c) >> [<c0267654>] (security_file_open) from [<c01854b0>] (do_dentry_open+0xc0/0x338) >> [<c01853f0>] (do_dentry_open) from [<c0185a38>] (vfs_open+0x54/0x58) >> [<c01859e4>] (vfs_open) from [<c0195480>] (do_last.isra.10+0x45c/0xcf8) >> [<c0195024>] (do_last.isra.10) from [<c0196140>] (path_openat+0x424/0x600) >> [<c0195d1c>] (path_openat) from [<c0197498>] (do_filp_open+0x3c/0x98) >> [<c019745c>] (do_filp_open) from [<c0186b44>] (do_sys_open+0x120/0x1e4) >> [<c0186a24>] (do_sys_open) from [<c0186c30>] (SyS_open+0x28/0x2c) >> [<c0186c08>] (SyS_open) from [<c0010200>] (__sys_trace_return+0x0/0x20) > > Yeah, good catch. > >> @@ -63,7 +64,9 @@ static int fanotify_get_response(struct fsnotify_group *group, >> >> pr_debug("%s: group=%p event=%p\n", __func__, group, event); >> >> - wait_event(group->fanotify_data.access_waitq, event->response); >> + while (!event->response) >> + wait_event_freezable(group->fanotify_data.access_waitq, >> + event->response); > > But if the process gets a signal while waiting, we will just livelock the > kernel in this loop as wait_event_freezable() will keep returning > ERESTARTSYS. So you need to be a bit more clever here... Hi Jack, Thanks for the quick review. To avoid livelock issue, is it fine to use below change? If agree, I will send v2 patch. @@ -63,7 +64,11 @@ static int fanotify_get_response(struct fsnotify_group *group, pr_debug("%s: group=%p event=%p\n", __func__, group, event); - wait_event(group->fanotify_data.access_waitq, event->response); + while (!event->response) { + if (wait_event_freezable(group->fanotify_data.access_waitq, + event->response)) + flush_signals(current); + } Thanks --------- Original Message --------- Sender : Jan Kara <jack@xxxxxxx> Date : 2018-02-16 15:59 (GMT+5:30) Title : Re: [PATCH] fanotify: allow freeze on suspend when waiting for response from userspace To : VIVEK TRIVEDI<t.vivek@xxxxxxxxxxx> CC : jack@xxxxxxx, amir73il@xxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, PANKAJ MISHRA<pankaj.m@xxxxxxxxxxx>, Kunal Shubham<k.shubham@xxxxxxxxxxx> On Fri 16-02-18 15:14:40, t.vivek@xxxxxxxxxxx wrote: > From: Vivek Trivedi <t.vivek@xxxxxxxxxxx> > > If fanotify userspace response server thread is frozen first, > it may fail to send response from userspace to kernel space listener. > In this scenario, fanotify response listener will never get response > from userepace and fail to suspend. > > Use freeze-friendly wait API to handle this issue. > > Same problem was reported here: > https://bbs.archlinux.org/viewtopic.php?id=232270 > > Freezing of tasks failed after 20.005 seconds > (1 tasks refusing to freeze, wq_busy=0) > > Backtrace: > [<c0582f80>] (__schedule) from [<c05835d0>] (schedule+0x4c/0xa4) > [<c0583584>] (schedule) from [<c01cb648>] (fanotify_handle_event+0x1c8/0x218) > [<c01cb480>] (fanotify_handle_event) from [<c01c8238>] (fsnotify+0x17c/0x38c) > [<c01c80bc>] (fsnotify) from [<c02676dc>] (security_file_open+0x88/0x8c) > [<c0267654>] (security_file_open) from [<c01854b0>] (do_dentry_open+0xc0/0x338) > [<c01853f0>] (do_dentry_open) from [<c0185a38>] (vfs_open+0x54/0x58) > [<c01859e4>] (vfs_open) from [<c0195480>] (do_last.isra.10+0x45c/0xcf8) > [<c0195024>] (do_last.isra.10) from [<c0196140>] (path_openat+0x424/0x600) > [<c0195d1c>] (path_openat) from [<c0197498>] (do_filp_open+0x3c/0x98) > [<c019745c>] (do_filp_open) from [<c0186b44>] (do_sys_open+0x120/0x1e4) > [<c0186a24>] (do_sys_open) from [<c0186c30>] (SyS_open+0x28/0x2c) > [<c0186c08>] (SyS_open) from [<c0010200>] (__sys_trace_return+0x0/0x20) Yeah, good catch. > @@ -63,7 +64,9 @@ static int fanotify_get_response(struct fsnotify_group *group, > > pr_debug("%s: group=%p event=%p\n", __func__, group, event); > > - wait_event(group->fanotify_data.access_waitq, event->response); > + while (!event->response) > + wait_event_freezable(group->fanotify_data.access_waitq, > + event->response); But if the process gets a signal while waiting, we will just livelock the kernel in this loop as wait_event_freezable() will keep returning ERESTARTSYS. So you need to be a bit more clever here... Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
