All of lore.kernel.org
 help / color / mirror / Atom feed
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 20:48:26 +0000	[thread overview]
Message-ID: <20190501204826.umekxc7oynslakes@dcvr> (raw)
In-Reply-To: <CABeXuvq7gCV2qPOo+Q8jvNyRaTvhkRLRbnL_oJ-AuK7Sp=P3QQ@mail.gmail.com>

Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> So here is my analysis:

<snip everything I agree with>

> So the 854a6ed56839a40f6 seems to be better than the original code in
> that it detects the signal.

OTOH, does matter to anybody that a signal is detected slightly
sooner than it would've been, otherwise?

> But, the problem is that it doesn't
> communicate it to the userspace.

Yup, that's a big problem :)
 
> So a patch like below solves the problem. This is incomplete. I'll
> verify and send you a proper fix you can test soon. This is just for
> the sake of discussion:
> 
> diff --git a/fs/eventpoll.c b/fs/eventpoll.c
> index 4a0e98d87fcc..63a387329c3d 100644
> --- a/fs/eventpoll.c
> +++ b/fs/eventpoll.c
> @@ -2317,7 +2317,7 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct
> epoll_event __user *, events,
>                 int, maxevents, int, timeout, const sigset_t __user *, sigmask,
>                 size_t, sigsetsize)
>  {
> -       int error;
> +       int error, signal_detected;
>         sigset_t ksigmask, sigsaved;
> 
>         /*
> @@ -2330,7 +2330,10 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct
> epoll_event __user *, events,
> 
>         error = do_epoll_wait(epfd, events, maxevents, timeout);
> 
> -       restore_user_sigmask(sigmask, &sigsaved);
> +       signal_detected = restore_user_sigmask(sigmask, &sigsaved);
> +
> +       if (signal_detected && !error)
> +               return -EITNR;
> 
>         return error;

Looks like a reasonable API.

> @@ -2862,7 +2862,7 @@ void restore_user_sigmask(const void __user
> *usigmask, sigset_t *sigsaved)
>         if (signal_pending(current)) {
>                 current->saved_sigmask = *sigsaved;
>                 set_restore_sigmask();
> -               return;
> +               return 0;

Shouldn't that "return 1" if a signal is pending?

  reply	other threads:[~2019-05-01 20:48 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
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 [this message]
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=20190501204826.umekxc7oynslakes@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.