From: Eric Wong <e@80x24.org>
To: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
Arnd Bergmann <arnd@arndb.de>, Al Viro <viro@zeniv.linux.org.uk>,
Jason Baron <jbaron@akamai.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Omar Kilani <omar.kilani@gmail.com>,
Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>
Subject: Re: Strange issues with epoll since 5.0
Date: Wed, 1 May 2019 02:14:05 +0000 [thread overview]
Message-ID: <20190501021405.hfvd7ps623liu25i@dcvr> (raw)
In-Reply-To: <CABeXuvqpAjk8ocRUabVU4Yviv7kgRkMneLE1Xy-jAtHdXAHBVw@mail.gmail.com>
Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> I was also not able to reproduce this.
> Arnd and I were talking about this today morning. Here is something
> Arnd noticed:
>
> If there was a signal after do_epoll_wait(), we never were not
> entering the if (err = -EINTR) at all before.
I'm not sure which `if' statement you're talking about, but ok...
> But, now we do.
> We could try with the below patch:
Wasn't close to applying or being buildable, but I put a
working version together (below).
epoll_pwait wakes up as expected, now :>
> If this works that means we know what is busted.
OK, good to know...
> I'm not sure what the hang in the userspace is about. Is it because
> the syscall did not return an error or the particular signal was
> blocked etc.
Uh, ok; that's less comforting.
> There are also a few timing differences also. But, can we try this first?
Anyways, the following patch works and builds cleanly for me
(didn't test AIO, but everything else seems good)
Thanks!
---------8<-------
Subject: [PATCH] test fix from Deepa
TBD
---
fs/aio.c | 8 ++++----
fs/eventpoll.c | 4 ++--
fs/select.c | 12 ++++++------
include/linux/signal.h | 2 +-
kernel/signal.c | 6 ++++--
5 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 3d9669d011b9..d54513ca11b6 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2146,7 +2146,7 @@ SYSCALL_DEFINE6(io_pgetevents,
return ret;
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
- restore_user_sigmask(ksig.sigmask, &sigsaved);
+ restore_user_sigmask(ksig.sigmask, &sigsaved, -1);
if (signal_pending(current) && !ret)
ret = -ERESTARTNOHAND;
@@ -2180,7 +2180,7 @@ SYSCALL_DEFINE6(io_pgetevents_time32,
return ret;
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
- restore_user_sigmask(ksig.sigmask, &sigsaved);
+ restore_user_sigmask(ksig.sigmask, &sigsaved, -1);
if (signal_pending(current) && !ret)
ret = -ERESTARTNOHAND;
@@ -2244,7 +2244,7 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents,
return ret;
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
- restore_user_sigmask(ksig.sigmask, &sigsaved);
+ restore_user_sigmask(ksig.sigmask, &sigsaved, -1);
if (signal_pending(current) && !ret)
ret = -ERESTARTNOHAND;
@@ -2277,7 +2277,7 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents_time64,
return ret;
ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
- restore_user_sigmask(ksig.sigmask, &sigsaved);
+ restore_user_sigmask(ksig.sigmask, &sigsaved, -1);
if (signal_pending(current) && !ret)
ret = -ERESTARTNOHAND;
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index a5d219d920e7..bd84ec54a8fb 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -2247,7 +2247,7 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events,
error = do_epoll_wait(epfd, events, maxevents, timeout);
- restore_user_sigmask(sigmask, &sigsaved);
+ restore_user_sigmask(sigmask, &sigsaved, error == -EINTR);
return error;
}
@@ -2272,7 +2272,7 @@ COMPAT_SYSCALL_DEFINE6(epoll_pwait, int, epfd,
err = do_epoll_wait(epfd, events, maxevents, timeout);
- restore_user_sigmask(sigmask, &sigsaved);
+ restore_user_sigmask(sigmask, &sigsaved, err == -EINTR);
return err;
}
diff --git a/fs/select.c b/fs/select.c
index d0f35dbc0e8f..04720e5856ed 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -760,7 +760,7 @@ static long do_pselect(int n, fd_set __user *inp, fd_set __user *outp,
ret = core_sys_select(n, inp, outp, exp, to);
ret = poll_select_copy_remaining(&end_time, tsp, type, ret);
- restore_user_sigmask(sigmask, &sigsaved);
+ restore_user_sigmask(sigmask, &sigsaved, -1);
return ret;
}
@@ -1106,7 +1106,7 @@ SYSCALL_DEFINE5(ppoll, struct pollfd __user *, ufds, unsigned int, nfds,
ret = do_sys_poll(ufds, nfds, to);
- restore_user_sigmask(sigmask, &sigsaved);
+ restore_user_sigmask(sigmask, &sigsaved, -1);
/* We can restart this syscall, usually */
if (ret == -EINTR)
@@ -1142,7 +1142,7 @@ SYSCALL_DEFINE5(ppoll_time32, struct pollfd __user *, ufds, unsigned int, nfds,
ret = do_sys_poll(ufds, nfds, to);
- restore_user_sigmask(sigmask, &sigsaved);
+ restore_user_sigmask(sigmask, &sigsaved, -1);
/* We can restart this syscall, usually */
if (ret == -EINTR)
@@ -1352,7 +1352,7 @@ static long do_compat_pselect(int n, compat_ulong_t __user *inp,
ret = compat_core_sys_select(n, inp, outp, exp, to);
ret = poll_select_copy_remaining(&end_time, tsp, type, ret);
- restore_user_sigmask(sigmask, &sigsaved);
+ restore_user_sigmask(sigmask, &sigsaved, -1);
return ret;
}
@@ -1425,7 +1425,7 @@ COMPAT_SYSCALL_DEFINE5(ppoll, struct pollfd __user *, ufds,
ret = do_sys_poll(ufds, nfds, to);
- restore_user_sigmask(sigmask, &sigsaved);
+ restore_user_sigmask(sigmask, &sigsaved, -1);
/* We can restart this syscall, usually */
if (ret == -EINTR)
@@ -1461,7 +1461,7 @@ COMPAT_SYSCALL_DEFINE5(ppoll_time64, struct pollfd __user *, ufds,
ret = do_sys_poll(ufds, nfds, to);
- restore_user_sigmask(sigmask, &sigsaved);
+ restore_user_sigmask(sigmask, &sigsaved, -1);
/* We can restart this syscall, usually */
if (ret == -EINTR)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 9702016734b1..b55804ae2021 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -276,7 +276,7 @@ extern int sigprocmask(int, sigset_t *, sigset_t *);
extern int set_user_sigmask(const sigset_t __user *usigmask, sigset_t *set,
sigset_t *oldset, size_t sigsetsize);
extern void restore_user_sigmask(const void __user *usigmask,
- sigset_t *sigsaved);
+ sigset_t *sigsaved, int sig_pending);
extern void set_current_blocked(sigset_t *);
extern void __set_current_blocked(const sigset_t *);
extern int show_unhandled_signals;
diff --git a/kernel/signal.c b/kernel/signal.c
index 57b7771e20d7..cc827e6c4bea 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2843,11 +2843,13 @@ EXPORT_SYMBOL(set_compat_user_sigmask);
* usigmask: sigmask passed in from userland.
* sigsaved: saved sigmask when the syscall started and changed the sigmask to
* usigmask.
+ * sig_pending: "> 0" if a signal is pending, "< 0" to check signal_pending
*
* This is useful for syscalls such as ppoll, pselect, io_pgetevents and
* epoll_pwait where a new sigmask is passed in from userland for the syscalls.
*/
-void restore_user_sigmask(const void __user *usigmask, sigset_t *sigsaved)
+void restore_user_sigmask(const void __user *usigmask, sigset_t *sigsaved,
+ int sig_pending)
{
if (!usigmask)
@@ -2857,7 +2859,7 @@ void restore_user_sigmask(const void __user *usigmask, sigset_t *sigsaved)
* Restoring sigmask here can lead to delivering signals that the above
* syscalls are intended to block because of the sigmask passed in.
*/
- if (signal_pending(current)) {
+ if (sig_pending > 0 || (sig_pending < 0 && signal_pending(current))) {
current->saved_sigmask = *sigsaved;
set_restore_sigmask();
return;
--
next prev parent reply other threads:[~2019-05-01 2:14 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-15 18:02 Strange issues with epoll since 5.0 Omar Kilani
2019-04-24 19:39 ` Eric Wong
2019-04-24 21:34 ` Davidlohr Bueso
2019-04-24 21:52 ` Davidlohr Bueso
2019-04-27 9:33 ` Eric Wong
2019-04-27 23:31 ` Deepa Dinamani
2019-04-28 0:48 ` Eric Wong
2019-04-29 20:47 ` Davidlohr Bueso
2019-04-29 21:04 ` Eric Wong
2019-04-30 21:07 ` Deepa Dinamani
2019-05-01 2:14 ` Eric Wong [this message]
2019-05-01 2:26 ` Eric Wong
2019-05-01 7:39 ` Eric Wong
2019-05-01 18:37 ` Deepa Dinamani
2019-05-01 20:48 ` Eric Wong
2019-05-01 20:53 ` Deepa Dinamani
2019-05-03 0:01 ` Deepa Dinamani
2019-05-03 2:34 ` Eric Wong
2019-05-03 3:34 ` Davidlohr Bueso
2019-05-03 3:42 ` [PATCH] signal: Adjust error codes according to restore_user_sigmask() Deepa Dinamani
2019-05-03 6:34 ` Eric Wong
2019-05-03 18:21 ` Deepa Dinamani
2019-05-03 19:51 ` Davidlohr Bueso
2019-05-03 22:53 ` Deepa Dinamani
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190501021405.hfvd7ps623liu25i@dcvr \
--to=e@80x24.org \
--cc=arnd@arndb.de \
--cc=dave@stgolabs.net \
--cc=deepa.kernel@gmail.com \
--cc=jbaron@akamai.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=omar.kilani@gmail.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).