Re: [PATCH v2 0/6] fanotify: Make wait for permission event response interruptible

[Jan Kara <jack@suse.cz>]

On Wed 20-02-19 10:27:04, Orion Poplawski wrote:
> On 2/13/19 7:54 AM, Jan Kara wrote:
> > Hello,
> > 
> > When waiting for response to fanotify permission events, we currently use
> > uninterruptible waits. That makes code simple however it can cause lots of
> > processes to end up in uninterruptible sleep with hard reboot being the only
> > alternative in case fanotify listener process stops responding (e.g. due to a
> > bug in its implementation) - reported e.g. in [1]. Uninterruptible sleep also
> > makes system hibernation fail if the listener gets frozen before the process
> > generating fanotify permission event (as reported e.g. here [2]).
> > 
> > This patch set modifies fanotify so that it will use interruptible wait when
> > waiting for fanotify permission event response. Patches are based on current
> > Linus' tree for the ease of testing (I plan to rebase them on top of Amir's
> > pending changes later). I have also create LTP test which stresses handling of
> > permission events while sending processes signals to test the new code [3]
> > Review, comments, and testing are welcome.
> > 
> > [1] https://lore.kernel.org/lkml/153474898224.6806.12518115530793064797.stgit@buzz/
> > [2] https://lore.kernel.org/lkml/c1bb16b7-9eee-9cea-2c96-a512d8b3b9c7@nwra.com/
> > [3] https://lwn.net/ml/linux-fsdevel/20190108165307.GA11259@quack2.suse.cz/
> > 
> > Changes since v1:
> > * leave pr_debug() calls alone (Amir)
> > * simplify permission event state tracking (Amir)
> > * split some changes into separate patches (Amir)
> > 
> > 								Honza
> > 
> 
> I backported these patches to the RHEL7 kernel and have started running that.
> One thing I've noticed are messages like the following at login time:
> 
> bash: /etc/bash_completion.d/itweb-settings.bash: Interrupted system call
> 
> I've commented on a bash bug report here
> https://savannah.gnu.org/support/?109159

Thanks for trying these out! Yes, so the patches can definitely lead to
EINTR returns from open(2) if there's fanotify permission event generated
by it and the opening process has a signal pending. Now EINTR is documented
as a possible return from open(2) but Marko is right that in practice
open(2) on local filesystem never returns EINTR so programs just don't
bother handling it. Since breaking userspace is no-go, we probably cannot
apply the change as is.

What we can do easily is to change the wait_event_interruptible() to
wait_event_killable(). This is what we commonly do when we want to allow
administrator to interrupt a syscall but userspace is not prepared for
EINTR. That will at least allow processes that are waiting for fanotify
response to be killed. So I'll do this for the coming merge window
(attached patch). However this will not solve your problems with
hibernation as TASK_KILLABLE tasks cannot be hibernated AFAICS. I will have
to talk with people more knowledgeable about hibernation if there's a
solution to this.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR