All of lore.kernel.org
 help / color / mirror / Atom feed
From: Davidlohr Bueso <dave@stgolabs.net>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: jbaron@akamai.com, rpenyaev@suse.de, viro@zeniv.linux.org.uk,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Davidlohr Bueso <dbueso@suse.de>
Subject: Re: [PATCH 2/2] fs/epoll: restore waking from ep_done_scan()
Date: Mon, 5 Apr 2021 20:22:26 -0700	[thread overview]
Message-ID: <20210406032226.2fpfzrlyxu2wz2jw@offworld> (raw)
In-Reply-To: <20210405185018.40d437d392863f743131fcda@linux-foundation.org>

On Mon, 05 Apr 2021, Andrew Morton wrote:

>Tricky.  339ddb53d373 was merged in December 2019.  So do we backport
>this fix?  Could any userspace code be depending upon the
>post-339ddb53d373 behavior?

As with previous trouble caused by this commit, I vote for restoring the behavior
backporting the fix, basically the equivalent of adding (which was my intention):

Fixes: 339ddb53d373 ("fs/epoll: remove unnecessary wakeups of nested epoll")

>
>Or do we just leave the post-339ddb53d373 code as-is?  Presumably the
>issue is very rarely encountered, and changeing it back has its own
>risks.

While I also consider this scenario rare (normally new ready events can become
ready and trigger new wakeups), I'm seeing reports in real applications of task
hangs due to this change of semantics. Alternatively, users can update their code
to timeout in such scenarios, but it is ultimately the kernel's fault. Furthermore
it hasn't really been all _that_ long since the commit was merged, so I don't think
it merits a change in behavior.

As for the risks of restoring the behavior, afaict this only fixed a double wakeup
in an obscure nested epoll scenario, so I'm not too worried there sacrificing
performance for functionality. That said, there are fixes, for example 65759097d80
(epoll: call final ep_events_available() check under the lock) that would perhaps
be rendered unnecessary.

Thanks,
Davidlohr

  reply	other threads:[~2021-04-06  3:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-05 23:10 [PATCH 0/2] fs/epoll: restore user-visible behavior upon event ready Davidlohr Bueso
2021-04-05 23:10 ` [PATCH 1/2] kselftest: introduce new epoll test case Davidlohr Bueso
2021-04-05 23:10 ` [PATCH 2/2] fs/epoll: restore waking from ep_done_scan() Davidlohr Bueso
2021-04-06  1:50   ` Andrew Morton
2021-04-06  3:22     ` Davidlohr Bueso [this message]
2021-04-06  5:09       ` Andrew Morton

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=20210406032226.2fpfzrlyxu2wz2jw@offworld \
    --to=dave@stgolabs.net \
    --cc=akpm@linux-foundation.org \
    --cc=dbueso@suse.de \
    --cc=jbaron@akamai.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rpenyaev@suse.de \
    --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.