Messages in this thread | | | Date | Fri, 10 Feb 2012 17:30:58 +0100 | From | Oleg Nesterov <> | Subject | Re: I finally prepared a testcase for read(inotify_fd) getting EINTR on PTRACE_ATTACH |
| |
On 02/10, Denys Vlasenko wrote: > > On 02/10/2012 04:09 PM, Oleg Nesterov wrote: >> On 02/10, Oleg Nesterov wrote: >>> At first glance this looks obvious? I never used inotify and I never >>> looked into fs/notify/inotify/, but it seems that inotify_read() simply >>> returns -EINTR if signal_pending() and doesn't implement restarts. >>> >>> Probably this trivial change >>> >>> --- x/fs/notify/inotify/inotify_user.c >>> +++ x/fs/notify/inotify/inotify_user.c >>> @@ -264,7 +264,7 @@ static ssize_t inotify_read(struct file >>> ret = -EAGAIN; >>> if (file->f_flags& O_NONBLOCK) >>> break; >>> - ret = -EINTR; >>> + ret = -ERESTARTSYS; >>> if (signal_pending(current)) >>> break; >>> >>> >>> makes sense. >> >> except I meant -ERESTARTNOHAND to avoid the behavioural change. > > I run-tested the fix. It works: testcase no longer fails > (modulo incorrect logic in the testcase which wase not working > properly on "no bug detected" code path. Fixed one: > http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/eintr-on-attach.c?cvsroot=systemtap > )
Good, thanks.
> I'm not sure inotify really wants to deviate from other reads > and return -EINTR even for SA_RESTARTing signals. IOW: > I think -ERESTARTSYS here would be more correct than -ERESTARTNOHAND.
I am not sure either. ERESTARTNOHAND doesn't change the behaviour, that was my point.
But I agree, ERESTARTSYS makes more sense to me.
Oleg.
| |