From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:56650 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751300AbdKFJPY (ORCPT ); Mon, 6 Nov 2017 04:15:24 -0500 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Jeremy Cline , Borislav Petkov , Thomas Gleixner , Andi Kleen , linux-edac@vger.kernel.org, Laura Abbott Subject: [PATCH 4.13 35/36] x86/mcelog: Get rid of RCU remnants Date: Mon, 6 Nov 2017 10:12:48 +0100 Message-Id: <20171106085048.608290620@linuxfoundation.org> In-Reply-To: <20171106085047.005824077@linuxfoundation.org> References: <20171106085047.005824077@linuxfoundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: stable-owner@vger.kernel.org List-ID: 4.13-stable review patch. If anyone has any objections, please let me know. ------------------ From: Borislav Petkov commit 7298f08ea8870d44d36c7d6cd07dd0303faef6c2 upstream. Jeremy reported a suspicious RCU usage warning in mcelog. /dev/mcelog is called in process context now as part of the notifier chain and doesn't need any of the fancy RCU and lockless accesses which it did in atomic context. Axe it all in favor of a simple mutex synchronization which cures the problem reported. Fixes: 5de97c9f6d85 ("x86/mce: Factor out and deprecate the /dev/mcelog driver") Reported-by: Jeremy Cline Signed-off-by: Borislav Petkov Signed-off-by: Thomas Gleixner Reviewed-and-tested-by: Tony Luck Cc: Andi Kleen Cc: linux-edac@vger.kernel.org Cc: Laura Abbott Link: https://lkml.kernel.org/r/20171101164754.xzzmskl4ngrqc5br@pd.tnic Link: https://bugzilla.redhat.com/show_bug.cgi?id=1498969 Signed-off-by: Greg Kroah-Hartman --- arch/x86/kernel/cpu/mcheck/dev-mcelog.c | 119 ++++++-------------------------- 1 file changed, 26 insertions(+), 93 deletions(-) --- a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c +++ b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c @@ -24,14 +24,6 @@ static DEFINE_MUTEX(mce_chrdev_read_mute static char mce_helper[128]; static char *mce_helper_argv[2] = { mce_helper, NULL }; -#define mce_log_get_idx_check(p) \ -({ \ - RCU_LOCKDEP_WARN(!rcu_read_lock_sched_held() && \ - !lockdep_is_held(&mce_chrdev_read_mutex), \ - "suspicious mce_log_get_idx_check() usage"); \ - smp_load_acquire(&(p)); \ -}) - /* * Lockless MCE logging infrastructure. * This avoids deadlocks on printk locks without having to break locks. Also @@ -53,43 +45,32 @@ static int dev_mce_log(struct notifier_b void *data) { struct mce *mce = (struct mce *)data; - unsigned int next, entry; + unsigned int entry; + + mutex_lock(&mce_chrdev_read_mutex); - wmb(); - for (;;) { - entry = mce_log_get_idx_check(mcelog.next); - for (;;) { - - /* - * When the buffer fills up discard new entries. - * Assume that the earlier errors are the more - * interesting ones: - */ - if (entry >= MCE_LOG_LEN) { - set_bit(MCE_OVERFLOW, - (unsigned long *)&mcelog.flags); - return NOTIFY_OK; - } - /* Old left over entry. Skip: */ - if (mcelog.entry[entry].finished) { - entry++; - continue; - } - break; - } - smp_rmb(); - next = entry + 1; - if (cmpxchg(&mcelog.next, entry, next) == entry) - break; + entry = mcelog.next; + + /* + * When the buffer fills up discard new entries. Assume that the + * earlier errors are the more interesting ones: + */ + if (entry >= MCE_LOG_LEN) { + set_bit(MCE_OVERFLOW, (unsigned long *)&mcelog.flags); + goto unlock; } + + mcelog.next = entry + 1; + memcpy(mcelog.entry + entry, mce, sizeof(struct mce)); - wmb(); mcelog.entry[entry].finished = 1; - wmb(); /* wake processes polling /dev/mcelog */ wake_up_interruptible(&mce_chrdev_wait); +unlock: + mutex_unlock(&mce_chrdev_read_mutex); + return NOTIFY_OK; } @@ -177,13 +158,6 @@ static int mce_chrdev_release(struct ino return 0; } -static void collect_tscs(void *data) -{ - unsigned long *cpu_tsc = (unsigned long *)data; - - cpu_tsc[smp_processor_id()] = rdtsc(); -} - static int mce_apei_read_done; /* Collect MCE record of previous boot in persistent storage via APEI ERST. */ @@ -231,14 +205,9 @@ static ssize_t mce_chrdev_read(struct fi size_t usize, loff_t *off) { char __user *buf = ubuf; - unsigned long *cpu_tsc; - unsigned prev, next; + unsigned next; int i, err; - cpu_tsc = kmalloc(nr_cpu_ids * sizeof(long), GFP_KERNEL); - if (!cpu_tsc) - return -ENOMEM; - mutex_lock(&mce_chrdev_read_mutex); if (!mce_apei_read_done) { @@ -247,65 +216,29 @@ static ssize_t mce_chrdev_read(struct fi goto out; } - next = mce_log_get_idx_check(mcelog.next); - /* Only supports full reads right now */ err = -EINVAL; if (*off != 0 || usize < MCE_LOG_LEN*sizeof(struct mce)) goto out; + next = mcelog.next; err = 0; - prev = 0; - do { - for (i = prev; i < next; i++) { - unsigned long start = jiffies; - struct mce *m = &mcelog.entry[i]; - - while (!m->finished) { - if (time_after_eq(jiffies, start + 2)) { - memset(m, 0, sizeof(*m)); - goto timeout; - } - cpu_relax(); - } - smp_rmb(); - err |= copy_to_user(buf, m, sizeof(*m)); - buf += sizeof(*m); -timeout: - ; - } - - memset(mcelog.entry + prev, 0, - (next - prev) * sizeof(struct mce)); - prev = next; - next = cmpxchg(&mcelog.next, prev, 0); - } while (next != prev); - synchronize_sched(); - - /* - * Collect entries that were still getting written before the - * synchronize. - */ - on_each_cpu(collect_tscs, cpu_tsc, 1); - - for (i = next; i < MCE_LOG_LEN; i++) { + for (i = 0; i < next; i++) { struct mce *m = &mcelog.entry[i]; - if (m->finished && m->tsc < cpu_tsc[m->cpu]) { - err |= copy_to_user(buf, m, sizeof(*m)); - smp_rmb(); - buf += sizeof(*m); - memset(m, 0, sizeof(*m)); - } + err |= copy_to_user(buf, m, sizeof(*m)); + buf += sizeof(*m); } + memset(mcelog.entry, 0, next * sizeof(struct mce)); + mcelog.next = 0; + if (err) err = -EFAULT; out: mutex_unlock(&mce_chrdev_read_mutex); - kfree(cpu_tsc); return err ? err : buf - ubuf; }