Hi Michael
> -----Original Message-----
> From: Michael S. Tsirkin <mst(a)redhat.com>
> Sent: Monday, May 4, 2020 8:16 PM
> To: Linus Torvalds <torvalds(a)linux-foundation.org>
> Cc: kvm(a)vger.kernel.org; virtualization(a)lists.linux-foundation.org;
> netdev(a)vger.kernel.org; linux-kernel(a)vger.kernel.org; Justin He
> <Justin.He(a)arm.com>; ldigby(a)redhat.com; mst(a)redhat.com; n.b(a)live.com;
> stefanha(a)redhat.com
> Subject: [GIT PULL] vhost: fixes
>
> The following changes since commit
> 6a8b55ed4056ea5559ebe4f6a4b247f627870d4c:
>
> Linux 5.7-rc3 (2020-04-26 13:51:02 -0700)
>
> are available in the Git repository at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
>
> for you to fetch changes up to
> 0b841030625cde5f784dd62aec72d6a766faae70:
>
> vhost: vsock: kick send_pkt worker once device is started (2020-05-02
> 10:28:21 -0400)
>
> ----------------------------------------------------------------
> virtio: fixes
>
> A couple of bug fixes.
>
> Signed-off-by: Michael S. Tsirkin <mst(a)redhat.com>
>
> ----------------------------------------------------------------
> Jia He (1):
> vhost: vsock: kick send_pkt worker once device is started
Should this fix also be CC-ed to stable? Sorry I forgot to cc it to stable.
--
Cheers,
Justin (Jia He)
>
> Stefan Hajnoczi (1):
> virtio-blk: handle block_device_operations callbacks after hot unplug
>
> drivers/block/virtio_blk.c | 86
> +++++++++++++++++++++++++++++++++++++++++-----
> drivers/vhost/vsock.c | 5 +++
> 2 files changed, 83 insertions(+), 8 deletions(-)
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
We recorded the dependencies for WAIT_FOR_SUBMIT in order that we could
correctly perform priority inheritance from the parallel branches to the
common trunk. However, for the purpose of timeslicing and reset
handling, the dependency is weak -- as we the pair of requests are
allowed to run in parallel and not in strict succession. So for example
we do need to suspend one if the other hangs.
The real significance though is that this allows us to rearrange
groups of WAIT_FOR_SUBMIT linked requests along the single engine, and
so can resolve user level inter-batch scheduling dependencies from user
semaphores.
Fixes: c81471f5e95c ("drm/i915: Copy across scheduler behaviour flags across submit fences")
Signed-off-by: Chris Wilson <chris(a)chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin(a)intel.com>
Cc: <stable(a)vger.kernel.org> # v5.6+
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 9 +++++++++
drivers/gpu/drm/i915/i915_request.c | 8 ++++++--
drivers/gpu/drm/i915/i915_scheduler.c | 4 +++-
drivers/gpu/drm/i915/i915_scheduler.h | 3 ++-
drivers/gpu/drm/i915/i915_scheduler_types.h | 1 +
5 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index dc3f2ee7136d..10109f661bcb 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1880,6 +1880,9 @@ static void defer_request(struct i915_request *rq, struct list_head * const pl)
struct i915_request *w =
container_of(p->waiter, typeof(*w), sched);
+ if (p->flags & I915_DEPENDENCY_WEAK)
+ continue;
+
/* Leave semaphores spinning on the other engines */
if (w->engine != rq->engine)
continue;
@@ -2726,6 +2729,9 @@ static void __execlists_hold(struct i915_request *rq)
struct i915_request *w =
container_of(p->waiter, typeof(*w), sched);
+ if (p->flags & I915_DEPENDENCY_WEAK)
+ continue;
+
/* Leave semaphores spinning on the other engines */
if (w->engine != rq->engine)
continue;
@@ -2850,6 +2856,9 @@ static void __execlists_unhold(struct i915_request *rq)
struct i915_request *w =
container_of(p->waiter, typeof(*w), sched);
+ if (p->flags & I915_DEPENDENCY_WEAK)
+ continue;
+
/* Propagate any change in error status */
if (rq->fence.error)
i915_request_set_error_once(w, rq->fence.error);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 22635bbabf06..95edc5523a01 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1038,7 +1038,9 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from)
return 0;
if (to->engine->schedule) {
- ret = i915_sched_node_add_dependency(&to->sched, &from->sched);
+ ret = i915_sched_node_add_dependency(&to->sched,
+ &from->sched,
+ 0);
if (ret < 0)
return ret;
}
@@ -1200,7 +1202,9 @@ __i915_request_await_execution(struct i915_request *to,
/* Couple the dependency tree for PI on this exposed to->fence */
if (to->engine->schedule) {
- err = i915_sched_node_add_dependency(&to->sched, &from->sched);
+ err = i915_sched_node_add_dependency(&to->sched,
+ &from->sched,
+ I915_DEPENDENCY_WEAK);
if (err < 0)
return err;
}
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 37cfcf5b321b..5f4c1e49e974 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -462,7 +462,8 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
}
int i915_sched_node_add_dependency(struct i915_sched_node *node,
- struct i915_sched_node *signal)
+ struct i915_sched_node *signal,
+ unsigned long flags)
{
struct i915_dependency *dep;
@@ -473,6 +474,7 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
local_bh_disable();
if (!__i915_sched_node_add_dependency(node, signal, dep,
+ flags |
I915_DEPENDENCY_EXTERNAL |
I915_DEPENDENCY_ALLOC))
i915_dependency_free(dep);
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index d1dc4efef77b..6f0bf00fc569 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -34,7 +34,8 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
unsigned long flags);
int i915_sched_node_add_dependency(struct i915_sched_node *node,
- struct i915_sched_node *signal);
+ struct i915_sched_node *signal,
+ unsigned long flags);
void i915_sched_node_fini(struct i915_sched_node *node);
diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h
index d18e70550054..7186875088a0 100644
--- a/drivers/gpu/drm/i915/i915_scheduler_types.h
+++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
@@ -78,6 +78,7 @@ struct i915_dependency {
unsigned long flags;
#define I915_DEPENDENCY_ALLOC BIT(0)
#define I915_DEPENDENCY_EXTERNAL BIT(1)
+#define I915_DEPENDENCY_WEAK BIT(2)
};
#endif /* _I915_SCHEDULER_TYPES_H_ */
--
2.20.1
This is the start of the stable review cycle for the 4.19.121 release.
There are 37 patches in this series, all will be posted as a response
to this one. If anyone has any issues with these being applied, please
let me know.
Responses should be made by Wed, 06 May 2020 16:52:55 +0000.
Anything received after that time might be too late.
The whole patch series can be found in one patch at:
https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.121-r…
or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.19.y
and the diffstat can be found below.
thanks,
greg k-h
-------------
Pseudo-Shortlog of commits:
Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Linux 4.19.121-rc1
Martin Blumenstingl <martin.blumenstingl(a)googlemail.com>
mmc: meson-mx-sdio: remove the broken ->card_busy() op
Martin Blumenstingl <martin.blumenstingl(a)googlemail.com>
mmc: meson-mx-sdio: Set MMC_CAP_WAIT_WHILE_BUSY
Veerabhadrarao Badiganti <vbadigan(a)codeaurora.org>
mmc: sdhci-msm: Enable host capabilities pertains to R1b response
Adrian Hunter <adrian.hunter(a)intel.com>
mmc: sdhci-pci: Fix eMMC driver strength for BYT-based controllers
Marek Behún <marek.behun(a)nic.cz>
mmc: sdhci-xenon: fix annoying 1.8V regulator warning
Douglas Anderson <dianders(a)chromium.org>
mmc: cqhci: Avoid false "cqhci: CQE stuck on" by not open-coding timeout loop
Qu Wenruo <wqu(a)suse.com>
btrfs: transaction: Avoid deadlock due to bad initialization timing of fs_info::journal_info
Filipe Manana <fdmanana(a)suse.com>
btrfs: fix partial loss of prealloc extent past i_size after fsync
Paul Moore <paul(a)paul-moore.com>
selinux: properly handle multiple messages in selinux_netlink_send()
Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
dmaengine: dmatest: Fix iteration non-stop logic
Andreas Gruenbacher <agruenba(a)redhat.com>
nfs: Fix potential posix_acl refcnt leak in nfs3_set_acl
Arnd Bergmann <arnd(a)arndb.de>
ALSA: opti9xx: shut up gcc-10 range warning
Suravee Suthikulpanit <suravee.suthikulpanit(a)amd.com>
iommu/amd: Fix legacy interrupt remapping for x2APIC-enabled system
David Disseldorp <ddiss(a)suse.de>
scsi: target/iblock: fix WRITE SAME zeroing
Tang Bin <tangbin(a)cmss.chinamobile.com>
iommu/qcom: Fix local_base status check
Sean Christopherson <sean.j.christopherson(a)intel.com>
vfio/type1: Fix VA->PA translation for PFNMAP VMAs in vaddr_get_pfn()
Yan Zhao <yan.y.zhao(a)intel.com>
vfio: avoid possible overflow in vfio_iommu_type1_pin_pages
Leon Romanovsky <leon(a)kernel.org>
RDMA/core: Fix race between destroy and release FD object
Leon Romanovsky <leon(a)kernel.org>
RDMA/core: Prevent mixed use of FDs between shared ufiles
Alaa Hleihel <alaa(a)mellanox.com>
RDMA/mlx4: Initialize ib_spec on the stack
Aharon Landau <aharonl(a)mellanox.com>
RDMA/mlx5: Set GRH fields in query QP on RoCE
Martin Wilck <mwilck(a)suse.com>
scsi: qla2xxx: check UNLOADING before posting async work
Martin Wilck <mwilck(a)suse.com>
scsi: qla2xxx: set UNLOADING before waiting for session deletion
Gabriel Krisman Bertazi <krisman(a)collabora.com>
dm multipath: use updated MPATHF_QUEUE_IO on mapping for bio-based mpath
Mikulas Patocka <mpatocka(a)redhat.com>
dm writecache: fix data corruption when reloading the target
Sunwook Eom <speed.eom(a)samsung.com>
dm verity fec: fix hash block number in verity_fec_decode
Dexuan Cui <decui(a)microsoft.com>
PM: hibernate: Freeze kernel threads in software_resume()
Kai-Heng Feng <kai.heng.feng(a)canonical.com>
PM: ACPI: Output correct message on target power state
Takashi Iwai <tiwai(a)suse.de>
ALSA: pcm: oss: Place the plugin buffer overflow checks correctly
Wu Bo <wubo40(a)huawei.com>
ALSA: hda/hdmi: fix without unlocked before return
Takashi Iwai <tiwai(a)suse.de>
ALSA: usb-audio: Correct a typo of NuPrime DAC-10 USB ID
Hui Wang <hui.wang(a)canonical.com>
ALSA: hda/realtek - Two front mics on a Lenovo ThinkCenter
Xiyu Yang <xiyuyang19(a)fudan.edu.cn>
btrfs: fix block group leak when removing fails
Vasily Averin <vvs(a)virtuozzo.com>
drm/qxl: qxl_release use after free
Vasily Averin <vvs(a)virtuozzo.com>
drm/qxl: qxl_release leak in qxl_hw_surface_alloc()
Vasily Averin <vvs(a)virtuozzo.com>
drm/qxl: qxl_release leak in qxl_draw_dirty_fb()
Ville Syrjälä <ville.syrjala(a)linux.intel.com>
drm/edid: Fix off-by-one in DispID DTD pixel clock
-------------
Diffstat:
Makefile | 4 +--
drivers/acpi/device_pm.c | 4 +--
drivers/dma/dmatest.c | 4 +--
drivers/gpu/drm/drm_edid.c | 2 +-
drivers/gpu/drm/qxl/qxl_cmd.c | 10 +++---
drivers/gpu/drm/qxl/qxl_display.c | 6 ++--
drivers/gpu/drm/qxl/qxl_draw.c | 13 +++----
drivers/gpu/drm/qxl/qxl_ioctl.c | 5 +--
drivers/infiniband/core/rdma_core.c | 4 +--
drivers/infiniband/hw/mlx4/main.c | 3 +-
drivers/infiniband/hw/mlx5/qp.c | 4 ++-
drivers/iommu/amd_iommu_init.c | 2 +-
drivers/iommu/qcom_iommu.c | 5 ++-
drivers/md/dm-mpath.c | 6 ++--
drivers/md/dm-verity-fec.c | 2 +-
drivers/md/dm-writecache.c | 52 +++++++++++++++++++--------
drivers/mmc/host/cqhci.c | 21 ++++++-----
drivers/mmc/host/meson-mx-sdio.c | 11 +-----
drivers/mmc/host/sdhci-msm.c | 2 ++
drivers/mmc/host/sdhci-pci-core.c | 3 ++
drivers/mmc/host/sdhci-xenon.c | 10 ++++++
drivers/scsi/qla2xxx/qla_os.c | 35 +++++++++----------
drivers/target/target_core_iblock.c | 2 +-
drivers/vfio/vfio_iommu_type1.c | 6 ++--
fs/btrfs/extent-tree.c | 16 +++++----
fs/btrfs/transaction.c | 13 +++++--
fs/btrfs/tree-log.c | 43 +++++++++++++++++++++--
fs/nfs/nfs3acl.c | 22 ++++++++----
kernel/power/hibernate.c | 7 ++++
security/selinux/hooks.c | 70 ++++++++++++++++++++++++-------------
sound/core/oss/pcm_plugin.c | 20 ++++++-----
sound/isa/opti9xx/miro.c | 9 +++--
sound/isa/opti9xx/opti92x-ad1848.c | 9 +++--
sound/pci/hda/patch_hdmi.c | 4 ++-
sound/pci/hda/patch_realtek.c | 1 +
sound/usb/quirks.c | 2 +-
36 files changed, 281 insertions(+), 151 deletions(-)
The original problem was described here:
https://lkml.org/lkml/2020/4/27/1121
There is a possible race when ep_scan_ready_list() leaves ->rdllist
and ->obflist empty for a short period of time although some events
are pending. It is quite likely that ep_events_available() observes
empty lists and goes to sleep. Since 339ddb53d373 ("fs/epoll: remove
unnecessary wakeups of nested epoll") we are conservative in wakeups
(there is only one place for wakeup and this is ep_poll_callback()),
thus ep_events_available() must always observe correct state of
two lists. The easiest and correct way is to do the final check
under the lock. This does not impact the performance, since lock
is taken anyway for adding a wait entry to the wait queue.
In this patch barrierless __set_current_state() is used. This is
safe since waitqueue_active() is called under the same lock on wakeup
side.
Short-circuit for fatal signals (i.e. fatal_signal_pending() check)
is moved to the line just before actual events harvesting routine.
This is fully compliant to what is said in the comment of the patch
where the actual fatal_signal_pending() check was added:
c257a340ede0 ("fs, epoll: short circuit fetching events if thread
has been killed").
Signed-off-by: Roman Penyaev <rpenyaev(a)suse.de>
Reported-by: Jason Baron <jbaron(a)akamai.com>
Cc: Andrew Morton <akpm(a)linux-foundation.org>
Cc: Khazhismel Kumykov <khazhy(a)google.com>
Cc: Alexander Viro <viro(a)zeniv.linux.org.uk>
Cc: linux-fsdevel(a)vger.kernel.org
Cc: linux-kernel(a)vger.kernel.org
Cc: stable(a)vger.kernel.org
---
fs/eventpoll.c | 48 ++++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index aba03ee749f8..8453e5403283 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1879,34 +1879,33 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
* event delivery.
*/
init_wait(&wait);
- write_lock_irq(&ep->lock);
- __add_wait_queue_exclusive(&ep->wq, &wait);
- write_unlock_irq(&ep->lock);
+ write_lock_irq(&ep->lock);
/*
- * We don't want to sleep if the ep_poll_callback() sends us
- * a wakeup in between. That's why we set the task state
- * to TASK_INTERRUPTIBLE before doing the checks.
+ * Barrierless variant, waitqueue_active() is called under
+ * the same lock on wakeup ep_poll_callback() side, so it
+ * is safe to avoid an explicit barrier.
*/
- set_current_state(TASK_INTERRUPTIBLE);
+ __set_current_state(TASK_INTERRUPTIBLE);
+
/*
- * Always short-circuit for fatal signals to allow
- * threads to make a timely exit without the chance of
- * finding more events available and fetching
- * repeatedly.
+ * Do the final check under the lock. ep_scan_ready_list()
+ * plays with two lists (->rdllist and ->ovflist) and there
+ * is always a race when both lists are empty for short
+ * period of time although events are pending, so lock is
+ * important.
*/
- if (fatal_signal_pending(current)) {
- res = -EINTR;
- break;
+ eavail = ep_events_available(ep);
+ if (!eavail) {
+ if (signal_pending(current))
+ res = -EINTR;
+ else
+ __add_wait_queue_exclusive(&ep->wq, &wait);
}
+ write_unlock_irq(&ep->lock);
- eavail = ep_events_available(ep);
- if (eavail)
- break;
- if (signal_pending(current)) {
- res = -EINTR;
+ if (eavail || res)
break;
- }
if (!schedule_hrtimeout_range(to, slack, HRTIMER_MODE_ABS)) {
timed_out = 1;
@@ -1927,6 +1926,15 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
}
send_events:
+ if (fatal_signal_pending(current))
+ /*
+ * Always short-circuit for fatal signals to allow
+ * threads to make a timely exit without the chance of
+ * finding more events available and fetching
+ * repeatedly.
+ */
+ res = -EINTR;
+
/*
* Try to transfer events to user space. In case we get 0 events and
* there's still timeout left over, we go trying again in search of
--
2.24.1
The patch titled
Subject: epoll: ensure ep_poll() doesn't miss wakeup events
has been removed from the -mm tree. Its filename was
epoll-ensure-ep_poll-doesnt-miss-wakeup-events.patch
This patch was dropped because it was withdrawn
------------------------------------------------------
From: Jason Baron <jbaron(a)akamai.com>
Subject: epoll: ensure ep_poll() doesn't miss wakeup events
Now that the ep_events_available() check is done in a lockless way, and we
no longer perform wakeups from ep_scan_ready_list(), we need to ensure
that either ep->rdllist has items or the overflow list is active. Prior
to: commit 339ddb53d373 ("fs/epoll: remove unnecessary wakeups of nested
epoll"), we did wake_up(&ep->wq) after manipulating the ep->rdllist and
the overflow list. Thus, any waiters would observe the correct state.
However, with that wake_up() now removed we need to be more careful to
ensure that condition.
Here's an example of what could go wrong:
We have epoll fds: epfd1, epfd2. And epfd1 is added to epfd2 and epfd2 is
added to a socket: epfd1->epfd2->socket. Thread a is doing epoll_wait()
on epfd1, and thread b is doing epoll_wait on epfd2. Then:
1) data comes in on socket
ep_poll_callback() wakes up threads a and b
2) thread a runs
ep_poll()
ep_scan_ready_list()
ep_send_events_proc()
ep_item_poll()
ep_scan_ready_list()
list_splice_init(&ep->rdllist, &txlist);
3) now thread b is running
ep_poll()
ep_events_available()
returns false
schedule_hrtimeout_range()
Thus, thread b has now scheduled and missed the wakeup.
Link: http://lkml.kernel.org/r/1588360533-11828-1-git-send-email-jbaron@akamai.com
Fixes: 339ddb53d373 ("fs/epoll: remove unnecessary wakeups of nested epoll")
Signed-off-by: Jason Baron <jbaron(a)akamai.com>
Reviewed-by: Roman Penyaev <rpenyaev(a)suse.de>
Cc: Alexander Viro <viro(a)zeniv.linux.org.uk>
Cc: Heiher <r(a)hev.cc>
Cc: Khazhismel Kumykov <khazhy(a)google.com>
Cc: Davidlohr Bueso <dbueso(a)suse.de>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
fs/eventpoll.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
--- a/fs/eventpoll.c~epoll-ensure-ep_poll-doesnt-miss-wakeup-events
+++ a/fs/eventpoll.c
@@ -704,8 +704,14 @@ static __poll_t ep_scan_ready_list(struc
* in a lockless way.
*/
write_lock_irq(&ep->lock);
- list_splice_init(&ep->rdllist, &txlist);
WRITE_ONCE(ep->ovflist, NULL);
+ /*
+ * In ep_poll() we use ep_events_available() in a lockless way to decide
+ * if events are available. So we need to preserve that either
+ * ep->oflist != EP_UNACTIVE_PTR or there are events on the ep->rdllist.
+ */
+ smp_wmb();
+ list_splice_init(&ep->rdllist, &txlist);
write_unlock_irq(&ep->lock);
/*
@@ -737,16 +743,21 @@ static __poll_t ep_scan_ready_list(struc
}
}
/*
+ * Quickly re-inject items left on "txlist".
+ */
+ list_splice(&txlist, &ep->rdllist);
+ /*
+ * In ep_poll() we use ep_events_available() in a lockless way to decide
+ * if events are available. So we need to preserve that either
+ * ep->oflist != EP_UNACTIVE_PTR or there are events on the ep->rdllist.
+ */
+ smp_wmb();
+ /*
* We need to set back ep->ovflist to EP_UNACTIVE_PTR, so that after
* releasing the lock, events will be queued in the normal way inside
* ep->rdllist.
*/
WRITE_ONCE(ep->ovflist, EP_UNACTIVE_PTR);
-
- /*
- * Quickly re-inject items left on "txlist".
- */
- list_splice(&txlist, &ep->rdllist);
__pm_relax(ep->ws);
write_unlock_irq(&ep->lock);
_
Patches currently in -mm which might be from jbaron(a)akamai.com are
The patch titled
Subject: epoll: call final ep_events_available() check under the lock
has been added to the -mm tree. Its filename is
epoll-call-final-ep_events_available-check-under-the-lock.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/epoll-call-final-ep_events_availab…
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/epoll-call-final-ep_events_availab…
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Roman Penyaev <rpenyaev(a)suse.de>
Subject: epoll: call final ep_events_available() check under the lock
There is a possible race when ep_scan_ready_list() leaves ->rdllist and
->obflist empty for a short period of time although some events are
pending. It is quite likely that ep_events_available() observes empty
lists and goes to sleep. Since 339ddb53d373 ("fs/epoll: remove
unnecessary wakeups of nested epoll") we are conservative in wakeups
(there is only one place for wakeup and this is ep_poll_callback()), thus
ep_events_available() must always observe correct state of two lists. The
easiest and correct way is to do the final check under the lock. This
does not impact the performance, since lock is taken anyway for adding a
wait entry to the wait queue.
The discussion of the problem can be found here:
https://lore.kernel.org/linux-fsdevel/a2f22c3c-c25a-4bda-8339-a7bdaf17849e@…
In this patch barrierless __set_current_state() is used. This is safe
since waitqueue_active() is called under the same lock on wakeup side.
Short-circuit for fatal signals (i.e. fatal_signal_pending() check) is
moved to the line just before actual events harvesting routine. This is
fully compliant to what is said in the comment of the patch where the
actual fatal_signal_pending() check was added: c257a340ede0 ("fs, epoll:
short circuit fetching events if thread has been killed").
Link: http://lkml.kernel.org/r/20200505145609.1865152-1-rpenyaev@suse.de
Fixes: 339ddb53d373 ("fs/epoll: remove unnecessary wakeups of nested epoll")
Signed-off-by: Roman Penyaev <rpenyaev(a)suse.de>
Reported-by: Jason Baron <jbaron(a)akamai.com>
Reviewed-by: Jason Baron <jbaron(a)akamai.com>
Cc: Khazhismel Kumykov <khazhy(a)google.com>
Cc: Alexander Viro <viro(a)zeniv.linux.org.uk>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
fs/eventpoll.c | 48 +++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)
--- a/fs/eventpoll.c~epoll-call-final-ep_events_available-check-under-the-lock
+++ a/fs/eventpoll.c
@@ -1890,34 +1890,33 @@ fetch_events:
* event delivery.
*/
init_wait(&wait);
- write_lock_irq(&ep->lock);
- __add_wait_queue_exclusive(&ep->wq, &wait);
- write_unlock_irq(&ep->lock);
+ write_lock_irq(&ep->lock);
/*
- * We don't want to sleep if the ep_poll_callback() sends us
- * a wakeup in between. That's why we set the task state
- * to TASK_INTERRUPTIBLE before doing the checks.
+ * Barrierless variant, waitqueue_active() is called under
+ * the same lock on wakeup ep_poll_callback() side, so it
+ * is safe to avoid an explicit barrier.
*/
- set_current_state(TASK_INTERRUPTIBLE);
+ __set_current_state(TASK_INTERRUPTIBLE);
+
/*
- * Always short-circuit for fatal signals to allow
- * threads to make a timely exit without the chance of
- * finding more events available and fetching
- * repeatedly.
+ * Do the final check under the lock. ep_scan_ready_list()
+ * plays with two lists (->rdllist and ->ovflist) and there
+ * is always a race when both lists are empty for short
+ * period of time although events are pending, so lock is
+ * important.
*/
- if (fatal_signal_pending(current)) {
- res = -EINTR;
- break;
+ eavail = ep_events_available(ep);
+ if (!eavail) {
+ if (signal_pending(current))
+ res = -EINTR;
+ else
+ __add_wait_queue_exclusive(&ep->wq, &wait);
}
+ write_unlock_irq(&ep->lock);
- eavail = ep_events_available(ep);
- if (eavail)
- break;
- if (signal_pending(current)) {
- res = -EINTR;
+ if (eavail || res)
break;
- }
if (!schedule_hrtimeout_range(to, slack, HRTIMER_MODE_ABS)) {
timed_out = 1;
@@ -1938,6 +1937,15 @@ fetch_events:
}
send_events:
+ if (fatal_signal_pending(current)) {
+ /*
+ * Always short-circuit for fatal signals to allow
+ * threads to make a timely exit without the chance of
+ * finding more events available and fetching
+ * repeatedly.
+ */
+ res = -EINTR;
+ }
/*
* Try to transfer events to user space. In case we get 0 events and
* there's still timeout left over, we go trying again in search of
_
Patches currently in -mm which might be from rpenyaev(a)suse.de are
kselftests-introduce-new-epoll60-testcase-for-catching-lost-wakeups.patch
epoll-atomically-remove-wait-entry-on-wake-up.patch
epoll-call-final-ep_events_available-check-under-the-lock.patch