From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751506AbdJOJlD (ORCPT ); Sun, 15 Oct 2017 05:41:03 -0400 Received: from mail.skyhub.de ([5.9.137.197]:35764 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750829AbdJOJlB (ORCPT ); Sun, 15 Oct 2017 05:41:01 -0400 Date: Sun, 15 Oct 2017 11:40:46 +0200 From: Borislav Petkov To: "Luck, Tony" Cc: Andi Kleen , Jeremy Cline , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , "x86@kernel.org" , "linux-edac@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Laura Abbott Subject: Re: x86/mce: suspicious RCU usage in 4.13.4 Message-ID: <20171015094046.6477zglnee3s75lc@pd.tnic> References: <9840bf0d-6756-75eb-1a2d-d3aace235244@redhat.com> <20171010194426.s7keveirclglx6vh@pd.tnic> <20171010200841.uhniuwc7pulqgmic@intel.com> <871smasqux.fsf@linux.intel.com> <20171011115020.i4gkcy5pjifh2zsm@pd.tnic> <3908561D78D1C84285E8C5FCA982C28F770038E5@ORSMSX114.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <3908561D78D1C84285E8C5FCA982C28F770038E5@ORSMSX114.amr.corp.intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 11, 2017 at 09:34:22PM +0000, Luck, Tony wrote: > > here's a second attempt at a more rigorous simplification: RCU stuff is > > gone and only a single loop scans through the elements. > > The dev_mce_log() changes look good now. > > You can apply the axe to more bits of mce_chrdev_read() though. Like that That provoked a very serious axing. Please check whether I went too far. Hunk below is ontop of what got axed already: --- diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c index 1e1c6d22c93e..17d2bab25720 100644 --- a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c +++ b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c @@ -162,13 +162,6 @@ static int mce_chrdev_release(struct inode *inode, struct file *file) 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. */ @@ -216,14 +209,9 @@ static ssize_t mce_chrdev_read(struct file *filp, char __user *ubuf, 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) { @@ -232,63 +220,32 @@ static ssize_t mce_chrdev_read(struct file *filp, char __user *ubuf, goto out; } - next = 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); - - /* - * 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)); - } + if (!m->finished) + continue; + + 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; } -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.