Re: pselect/etc semantics

[Deepa Dinamani]

On Thu, May 30, 2019 at 8:48 AM Deepa Dinamani <deepa.kernel@xxxxxxxxx> wrote:
>
> > On May 30, 2019, at 8:38 AM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
> >
> > ebiederm@xxxxxxxxxxxx (Eric W. Biederman) writes:
> >
> >> Which means I believe we have a semantically valid change in behavior
> >> that is causing a regression.
> >
> > I haven't made a survey of all of the functions yet but
> > fucntions return -ENORESTARTNOHAND will never return -EINTR and are
> > immune from this problem.
> >
> > AKA pselect is fine.  While epoll_pwait can be affected.
>
> This was my understanding as well.

I think I was misremembered here. I had noted this before:
https://lore.kernel.org/linux-fsdevel/CABeXuvq7gCV2qPOo+Q8jvNyRaTvhkRLRbnL_oJ-AuK7Sp=P3QQ@xxxxxxxxxxxxxx/

"sys_io_pgetevents() does not seem to have this problem as we are still
checking signal_pending() here.
sys_pselect6() seems to have a similar problem. The changes to
sys_pselect6() also impact sys_select() as the changes are in the
common code path."

This was the code replaced for io_pgetevents by 854a6ed56839a40f6b is as below.
No matter what events completed, there was signal_pending() check
after the return from do_io_getevents().

--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2110,18 +2110,9 @@ SYSCALL_DEFINE6(io_pgetevents,
                return ret;

        ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
-       if (signal_pending(current)) {
-               if (ksig.sigmask) {
-                       current->saved_sigmask = sigsaved;
-                       set_restore_sigmask();
-               }
-
-               if (!ret)
-                       ret = -ERESTARTNOHAND;
-       } else {
-               if (ksig.sigmask)
-                       sigprocmask(SIG_SETMASK, &sigsaved, NULL);
-       }
+       restore_user_sigmask(ksig.sigmask, &sigsaved);
+       if (signal_pending(current) && !ret)
+               ret = -ERESTARTNOHAND;

Can I ask a simple question for my understanding?

man page for epoll_pwait says

EINTR
The call was interrupted by a signal handler before either any of the
requested events occurred or the timeout expired; see signal(7).

But it is not clear to me if we can figure out(without race) the
chronological order if one of the requested events are completed or a
signal came first.
Is this a correct exectation?

Also like pointed out above, this behavior is not consistent for all
such syscalls(io_pgetevents). Was this also by design?

-Deepa