linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Sandeep Patil <sspatil@android.com>,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 5.10 03/67] pipe: make pipe writes always wake up readers
Date: Mon,  2 Aug 2021 15:44:26 +0200	[thread overview]
Message-ID: <20210802134339.131493909@linuxfoundation.org> (raw)
In-Reply-To: <20210802134339.023067817@linuxfoundation.org>

From: Linus Torvalds <torvalds@linux-foundation.org>

commit 3a34b13a88caeb2800ab44a4918f230041b37dd9 upstream.

Since commit 1b6b26ae7053 ("pipe: fix and clarify pipe write wakeup
logic") we have sanitized the pipe write logic, and would only try to
wake up readers if they needed it.

In particular, if the pipe already had data in it before the write,
there was no point in trying to wake up a reader, since any existing
readers must have been aware of the pre-existing data already.  Doing
extraneous wakeups will only cause potential thundering herd problems.

However, it turns out that some Android libraries have misused the EPOLL
interface, and expected "edge triggered" be to "any new write will
trigger it".  Even if there was no edge in sight.

Quoting Sandeep Patil:
 "The commit 1b6b26ae7053 ('pipe: fix and clarify pipe write wakeup
  logic') changed pipe write logic to wakeup readers only if the pipe
  was empty at the time of write. However, there are libraries that
  relied upon the older behavior for notification scheme similar to
  what's described in [1]

  One such library 'realm-core'[2] is used by numerous Android
  applications. The library uses a similar notification mechanism as GNU
  Make but it never drains the pipe until it is full. When Android moved
  to v5.10 kernel, all applications using this library stopped working.

  The library has since been fixed[3] but it will be a while before all
  applications incorporate the updated library"

Our regression rule for the kernel is that if applications break from
new behavior, it's a regression, even if it was because the application
did something patently wrong.  Also note the original report [4] by
Michal Kerrisk about a test for this epoll behavior - but at that point
we didn't know of any actual broken use case.

So add the extraneous wakeup, to approximate the old behavior.

[ I say "approximate", because the exact old behavior was to do a wakeup
  not for each write(), but for each pipe buffer chunk that was filled
  in. The behavior introduced by this change is not that - this is just
  "every write will cause a wakeup, whether necessary or not", which
  seems to be sufficient for the broken library use. ]

It's worth noting that this adds the extraneous wakeup only for the
write side, while the read side still considers the "edge" to be purely
about reading enough from the pipe to allow further writes.

See commit f467a6a66419 ("pipe: fix and clarify pipe read wakeup logic")
for the pipe read case, which remains that "only wake up if the pipe was
full, and we read something from it".

Link: https://lore.kernel.org/lkml/CAHk-=wjeG0q1vgzu4iJhW5juPkTsjTYmiqiMUYAebWW+0bam6w@mail.gmail.com/ [1]
Link: https://github.com/realm/realm-core [2]
Link: https://github.com/realm/realm-core/issues/4666 [3]
Link: https://lore.kernel.org/lkml/CAKgNAkjMBGeAwF=2MKK758BhxvW58wYTgYKB2V-gY1PwXxrH+Q@mail.gmail.com/ [4]
Link: https://lore.kernel.org/lkml/20210729222635.2937453-1-sspatil@android.com/
Reported-by: Sandeep Patil <sspatil@android.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/pipe.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -429,20 +429,20 @@ pipe_write(struct kiocb *iocb, struct io
 #endif
 
 	/*
-	 * Only wake up if the pipe started out empty, since
-	 * otherwise there should be no readers waiting.
+	 * Epoll nonsensically wants a wakeup whether the pipe
+	 * was already empty or not.
 	 *
 	 * If it wasn't empty we try to merge new data into
 	 * the last buffer.
 	 *
 	 * That naturally merges small writes, but it also
-	 * page-aligs the rest of the writes for large writes
+	 * page-aligns the rest of the writes for large writes
 	 * spanning multiple pages.
 	 */
 	head = pipe->head;
-	was_empty = pipe_empty(head, pipe->tail);
+	was_empty = true;
 	chars = total_len & (PAGE_SIZE-1);
-	if (chars && !was_empty) {
+	if (chars && !pipe_empty(head, pipe->tail)) {
 		unsigned int mask = pipe->ring_size - 1;
 		struct pipe_buffer *buf = &pipe->bufs[(head - 1) & mask];
 		int offset = buf->offset + buf->len;



  parent reply	other threads:[~2021-08-02 14:00 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02 13:44 [PATCH 5.10 00/67] 5.10.56-rc1 review Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 01/67] io_uring: fix null-ptr-deref in io_sq_offload_start() Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 02/67] x86/asm: Ensure asm/proto.h can be included stand-alone Greg Kroah-Hartman
2021-08-02 13:44 ` Greg Kroah-Hartman [this message]
2021-08-02 13:44 ` [PATCH 5.10 04/67] btrfs: fix rw device counting in __btrfs_free_extra_devids Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 05/67] btrfs: mark compressed range uptodate only if all bio succeed Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 06/67] Revert "ACPI: resources: Add checks for ACPI IRQ override" Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 07/67] ACPI: DPTF: Fix reading of attributes Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 08/67] x86/kvm: fix vcpu-id indexed array sizes Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 09/67] KVM: add missing compat KVM_CLEAR_DIRTY_LOG Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 10/67] ocfs2: fix zero out valid data Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 11/67] ocfs2: issue zeroout to EOF blocks Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 12/67] can: j1939: j1939_xtp_rx_dat_one(): fix rxtimer value between consecutive TP.DT to 750ms Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 13/67] can: raw: raw_setsockopt(): fix raw_rcv panic for sock UAF Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 14/67] can: peak_usb: pcan_usb_handle_bus_evt(): fix reading rxerr/txerr values Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 15/67] can: mcba_usb_start(): add missing urb->transfer_dma initialization Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 16/67] can: usb_8dev: fix memory leak Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 17/67] can: ems_usb: " Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 18/67] can: esd_usb2: " Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 19/67] alpha: register early reserved memory in memblock Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 20/67] HID: wacom: Re-enable touch by default for Cintiq 24HDT / 27QHDT Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 21/67] NIU: fix incorrect error return, missed in previous revert Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 22/67] drm/amd/display: ensure dentist display clock update finished in DCN20 Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 23/67] drm/amdgpu: Avoid printing of stack contents on firmware load error Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 24/67] drm/amdgpu: Fix resource leak on probe error path Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 25/67] blk-iocost: fix operation ordering in iocg_wake_fn() Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 26/67] nfc: nfcsim: fix use after free during module unload Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 27/67] cfg80211: Fix possible memory leak in function cfg80211_bss_update Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 28/67] RDMA/bnxt_re: Fix stats counters Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 29/67] bpf: Fix OOB read when printing XDP link fdinfo Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 30/67] mac80211: fix enabling 4-address mode on a sta vif after assoc Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 31/67] netfilter: conntrack: adjust stop timestamp to real expiry value Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 32/67] netfilter: nft_nat: allow to specify layer 4 protocol NAT only Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 33/67] i40e: Fix logic of disabling queues Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 34/67] i40e: Fix firmware LLDP agent related warning Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 35/67] i40e: Fix queue-to-TC mapping on Tx Greg Kroah-Hartman
2021-08-02 13:44 ` [PATCH 5.10 36/67] i40e: Fix log TC creation failure when max num of queues is exceeded Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 37/67] tipc: fix implicit-connect for SYN+ Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 38/67] tipc: fix sleeping in tipc accept routine Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 39/67] net: Set true network header for ECN decapsulation Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 40/67] net: qrtr: fix memory leaks Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 41/67] ionic: remove intr coalesce update from napi Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 42/67] ionic: fix up dim accounting for tx and rx Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 43/67] ionic: count csum_none when offload enabled Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 44/67] tipc: do not write skb_shinfo frags when doing decrytion Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 45/67] octeontx2-pf: Fix interface down flag on error Greg Kroah-Hartman
2021-08-02 17:40   ` Pavel Machek
2021-08-02 13:45 ` [PATCH 5.10 46/67] mlx4: Fix missing error code in mlx4_load_one() Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 47/67] KVM: x86: Check the right feature bit for MSR_KVM_ASYNC_PF_ACK access Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 48/67] net: llc: fix skb_over_panic Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 49/67] drm/msm/dpu: Fix sm8250_mdp register length Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 50/67] drm/msm/dp: Initialize the INTF_CONFIG register Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 51/67] skmsg: Make sk_psock_destroy() static Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 52/67] net/mlx5: Fix flow table chaining Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 53/67] net/mlx5e: Fix nullptr in mlx5e_hairpin_get_mdev() Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 54/67] sctp: fix return value check in __sctp_rcv_asconf_lookup Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 55/67] tulip: windbond-840: Fix missing pci_disable_device() in probe and remove Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 56/67] sis900: " Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 57/67] can: hi311x: fix a signedness bug in hi3110_cmd() Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 58/67] bpf: Introduce BPF nospec instruction for mitigating Spectre v4 Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 59/67] bpf: Fix leakage due to insufficient speculative store bypass mitigation Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 60/67] bpf: Remove superfluous aux sanitation on subprog rejection Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 61/67] bpf: verifier: Allocate idmap scratch in verifier env Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 62/67] bpf: Fix pointer arithmetic mask tightening under state pruning Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 63/67] SMB3: fix readpage for large swap cache Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 64/67] powerpc/pseries: Fix regression while building external modules Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 65/67] Revert "perf map: Fix dso->nsinfo refcounting" Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 66/67] i40e: Add additional info to PHY type error Greg Kroah-Hartman
2021-08-02 13:45 ` [PATCH 5.10 67/67] can: j1939: j1939_session_deactivate(): clarify lifetime of session object Greg Kroah-Hartman
2021-08-02 16:23 ` [PATCH 5.10 00/67] 5.10.56-rc1 review Fox Chen
2021-08-02 17:49 ` Pavel Machek
2021-08-03  7:23 ` Naresh Kamboju
2021-08-03 10:38 ` Sudip Mukherjee
2021-08-03 11:13 ` Rudi Heitbaum
2021-08-03 19:16 ` Guenter Roeck
2021-08-03 19:26 ` Pavel Machek
2021-08-03 19:37   ` Guenter Roeck
2021-08-03 19:50     ` Pavel Machek
2021-08-03 20:52       ` Guenter Roeck
2021-08-04  6:21   ` Greg Kroah-Hartman
2021-08-04  6:25   ` Thomas Backlund
2021-08-04  6:35     ` Greg Kroah-Hartman
2021-08-04  3:05 ` Samuel Zou

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=20210802134339.131493909@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=sspatil@android.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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).