Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1258205
| From | Jiri Slaby <jslaby@suse.cz> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH 3.12 004/123] rcu: Reject memory-order-induced stall-warning false positives |
| Date | 2015-10-28 15:50 +0100 |
| Message-ID | <qoAd3-3T1-1@gated-at.bofh.it> (permalink) |
| References | <qozqF-3k6-9@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
3.12-stable review patch. If anyone has any objections, please let me know.
===============
commit 26cdfedf6a902345f8604ea8e0b7dd2566b37a46 upstream.
If a system is idle from an RCU perspective for longer than specified
by CONFIG_RCU_CPU_STALL_TIMEOUT, and if one CPU starts a grace period
just as a second checks for CPU stalls, and if this second CPU happens
to see the old value of rsp->jiffies_stall, it will incorrectly report a
CPU stall. This is quite rare, but apparently occurs deterministically
on systems with about 6TB of memory.
This commit therefore orders accesses to the data used to determine
whether or not a CPU stall is in progress. Grace-period initialization
and cleanup first increments rsp->completed to mark the end of the
previous grace period, then records the current jiffies in rsp->gp_start,
then records the jiffies at which a stall can be expected to occur in
rsp->jiffies_stall, and finally increments rsp->gpnum to mark the start
of the new grace period. Now, this ordering by itself does not prevent
false positives. For example, if grace-period initialization was delayed
between recording rsp->gp_start and rsp->jiffies_stall, the CPU stall
warning code might still see an old value of rsp->jiffies_stall.
Therefore, this commit also orders the CPU stall warning accesses as
well, loading rsp->gpnum and jiffies, then rsp->jiffies_stall, then
rsp->gp_start, and finally rsp->completed. This ordering means that
the false-positive scenario in the previous paragraph would result
in rsp->completed being greater than or equal to rsp->gpnum, which is
never valid for a CPU stall, allowing the false positive to be rejected.
Furthermore, any fetch that gets an old value of rsp->jiffies_stall
must also get an old value of rsp->gpnum, which will again be rejected
by the comparison of rsp->gpnum and rsp->completed. Situations where
rsp->gp_start is later than rsp->jiffies_stall are also rejected, as
are situations where jiffies is less than rsp->jiffies_stall.
Although use of unsynchronized accesses means that there are likely
still some false-positive scenarios (synchronization has proven to be
a very bad idea on large systems), this should get rid of a large class
of these scenarios.
Reported-by: Fabian Herschel <fabian.herschel@suse.com>
Reported-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Michal Hocko <mhocko@suse.cz>
Tested-by: Jochen Striepe <jochen@tolot.escape.de>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
kernel/rcutree.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 5 deletions(-)
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index e27526232b5f..a92bd6bd2bf1 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -802,8 +802,11 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
static void record_gp_stall_check_time(struct rcu_state *rsp)
{
- rsp->gp_start = jiffies;
- rsp->jiffies_stall = jiffies + rcu_jiffies_till_stall_check();
+ unsigned long j = ACCESS_ONCE(jiffies);
+
+ rsp->gp_start = j;
+ smp_wmb(); /* Record start time before stall time. */
+ rsp->jiffies_stall = j + rcu_jiffies_till_stall_check();
}
/*
@@ -932,17 +935,48 @@ static void print_cpu_stall(struct rcu_state *rsp)
static void check_cpu_stall(struct rcu_state *rsp, struct rcu_data *rdp)
{
+ unsigned long completed;
+ unsigned long gpnum;
+ unsigned long gps;
unsigned long j;
unsigned long js;
struct rcu_node *rnp;
- if (rcu_cpu_stall_suppress)
+ if (rcu_cpu_stall_suppress || !rcu_gp_in_progress(rsp))
return;
j = ACCESS_ONCE(jiffies);
+
+ /*
+ * Lots of memory barriers to reject false positives.
+ *
+ * The idea is to pick up rsp->gpnum, then rsp->jiffies_stall,
+ * then rsp->gp_start, and finally rsp->completed. These values
+ * are updated in the opposite order with memory barriers (or
+ * equivalent) during grace-period initialization and cleanup.
+ * Now, a false positive can occur if we get an new value of
+ * rsp->gp_start and a old value of rsp->jiffies_stall. But given
+ * the memory barriers, the only way that this can happen is if one
+ * grace period ends and another starts between these two fetches.
+ * Detect this by comparing rsp->completed with the previous fetch
+ * from rsp->gpnum.
+ *
+ * Given this check, comparisons of jiffies, rsp->jiffies_stall,
+ * and rsp->gp_start suffice to forestall false positives.
+ */
+ gpnum = ACCESS_ONCE(rsp->gpnum);
+ smp_rmb(); /* Pick up ->gpnum first... */
js = ACCESS_ONCE(rsp->jiffies_stall);
+ smp_rmb(); /* ...then ->jiffies_stall before the rest... */
+ gps = ACCESS_ONCE(rsp->gp_start);
+ smp_rmb(); /* ...and finally ->gp_start before ->completed. */
+ completed = ACCESS_ONCE(rsp->completed);
+ if (ULONG_CMP_GE(completed, gpnum) ||
+ ULONG_CMP_LT(j, js) ||
+ ULONG_CMP_GE(gps, js))
+ return; /* No stall or GP completed since entering function. */
rnp = rdp->mynode;
if (rcu_gp_in_progress(rsp) &&
- (ACCESS_ONCE(rnp->qsmask) & rdp->grpmask) && ULONG_CMP_GE(j, js)) {
+ (ACCESS_ONCE(rnp->qsmask) & rdp->grpmask)) {
/* We haven't checked in, so go dump stack. */
print_cpu_stall(rsp);
@@ -1331,9 +1365,10 @@ static int rcu_gp_init(struct rcu_state *rsp)
}
/* Advance to a new grace period and initialize state. */
+ record_gp_stall_check_time(rsp);
+ smp_wmb(); /* Record GP times before starting GP. */
rsp->gpnum++;
trace_rcu_grace_period(rsp->name, rsp->gpnum, TPS("start"));
- record_gp_stall_check_time(rsp);
raw_spin_unlock_irq(&rnp->lock);
/* Exclude any concurrent CPU-hotplug operations. */
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 3.12 001/123] x86/nmi/64: Fix a paravirt stack-clobbering bug in the NMI code Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:00 +0100
[PATCH 3.12 027/123] ARM: dts: omap5-uevm.dts: fix i2c5 pinctrl offsets Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:00 +0100
[PATCH 3.12 035/123] arc,hexagon: Delete asm/barrier.h Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:00 +0100
[PATCH 3.12 037/123] spi: Fix documentation of spi_alloc_master() Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:00 +0100
[PATCH 3.12 029/123] x86/apic: Serialize LVTT and TSC_DEADLINE writes Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:00 +0100
[PATCH 3.12 026/123] windfarm: decrement client count when unregistering Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:00 +0100
[PATCH 3.12 021/123] perf stat: Get correct cpu id for print_aggr Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
[PATCH 3.12 030/123] x86/platform: Fix Geode LX timekeeping in the generic x86 build Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
[PATCH 3.12 031/123] x86/mm: Set NX on gap between __ex_table and rodata Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
[PATCH 3.12 033/123] arch: Clean up asm/barrier.h implementations using asm-generic/barrier.h Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
[PATCH 3.12 032/123] x86/xen: Support kexec/kdump in HVM guests by doing a soft reset Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
Re: [PATCH 3.12 032/123] x86/xen: Support kexec/kdump in HVM guests by doing a soft reset Luis Henriques <luis.henriques@canonical.com> - 2015-10-28 16:30 +0100
Re: [PATCH 3.12 032/123] x86/xen: Support kexec/kdump in HVM guests by doing a soft reset Jiri Slaby <jslaby@suse.cz> - 2015-10-28 16:40 +0100
[PATCH 3.12 022/123] perf header: Fixup reading of HEADER_NRCPUS feature Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
[PATCH 3.12 034/123] arch: Move smp_mb__{before,after}_atomic_{inc,dec}.h into asm/atomic.h Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
[PATCH 3.12 025/123] ARM: 8429/1: disable GCC SRA optimization Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
[PATCH 3.12 028/123] dmaengine: dw: properly read DWC_PARAMS register Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:30 +0100
[PATCH 3.12 014/123] ARM: 7880/1: Clear the IT state independent of the Thumb-2 mode Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 036/123] sched/core: Fix TASK_DEAD race in finish_task_switch() Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 012/123] s390/3270: redraw screen on unsolicited device end Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 013/123] Use WARN_ON_ONCE for missing X86_FEATURE_NRIPS Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 023/123] hwmon: (nct6775) Swap STEP_UP_TIME and STEP_DOWN_TIME registers for most chips Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 017/123] kvm: fix zero length mmio searching Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 018/123] scsi: fix scsi_error_handler vs. scsi_host_dev_release race Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 002/123] x86/paravirt: Replace the paravirt nop with a bona fide empty function Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 020/123] perf hists: Update the column width for the "srcline" sort key Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 019/123] iser-target: remove command with state ISTATE_REMOVE Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 024/123] ARM: fix Thumb2 signal handling when ARMv6 is enabled Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 016/123] ALSA: hda - Control SPDIF out pin on MacBookPro 11,2 Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 015/123] net: add length argument to skb_copy_and_csum_datagram_iovec Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:40 +0100
[PATCH 3.12 004/123] rcu: Reject memory-order-induced stall-warning false positives Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
[PATCH 3.12 005/123] sched: Fix cpu_active_mask/cpu_online_mask race Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
[PATCH 3.12 007/123] xhci: rework cycle bit checking for new dequeue pointers Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
[PATCH 3.12 006/123] xhci: Workaround for PME stuck issues in Intel xhci Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
[PATCH 3.12 009/123] USB: usbtmc: add device quirk for Rigol DS6104 Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
Re: [PATCH 3.12 009/123] USB: usbtmc: add device quirk for Rigol DS6104 Teunis van Beelen <teuniz@gmail.com> - 2015-10-28 16:40 +0100
[PATCH 3.12 008/123] usb: core: Fix USB 3.0 devices lost in NOTATTACHED state after a hub port reset Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
[PATCH 3.12 010/123] client MUST ignore EncryptionKeyLength if CAP_EXTENDED_SECURITY is set Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
[PATCH 3.12 011/123] iommu/amd: Handle integer overflow in dma_ops_area_alloc Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
[PATCH 3.12 003/123] jbd2: avoid infinite loop when destroying aborted journal Jiri Slaby <jslaby@suse.cz> - 2015-10-28 15:50 +0100
csiph-web