Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759073AbcLPTXS (ORCPT ); Fri, 16 Dec 2016 14:23:18 -0500 Received: from mail-vk0-f44.google.com ([209.85.213.44]:35266 "EHLO mail-vk0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758477AbcLPTXR (ORCPT ); Fri, 16 Dec 2016 14:23:17 -0500 MIME-Version: 1.0 In-Reply-To: References: <20161215162648.061449202@linutronix.de> <20161215164240.813682510@linutronix.de> From: Andy Lutomirski Date: Fri, 16 Dec 2016 11:22:55 -0800 Message-ID: Subject: Re: [patch 2/3] x86/process: Optimize TIF_BLOCKSTEP switch To: Thomas Gleixner Cc: LKML , X86 ML , Peter Zijlstra , Kyle Huey , Andy Lutomirski Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1565 Lines: 41 On Fri, Dec 16, 2016 at 12:47 AM, Thomas Gleixner wrote: > On Thu, 15 Dec 2016, Andy Lutomirski wrote: >> On Thu, Dec 15, 2016 at 8:44 AM, Thomas Gleixner wrote: >> > +static inline void toggle_debugctlmsr(unsigned long mask) >> > +{ >> > + unsigned long msrval; >> > + >> > +#ifndef CONFIG_X86_DEBUGCTLMSR >> > + if (boot_cpu_data.x86 < 6) >> > + return; >> > +#endif >> > + rdmsrl(MSR_IA32_DEBUGCTLMSR, msrval); >> > + wrmsrl(MSR_IA32_DEBUGCTLMSR, msrval ^ mask); >> > +} >> > + >> >> This scares me. If the MSR ever gets out of sync with the TI flag, >> this will malfunction. And IIRC the MSR is highly magical and the CPU >> clears it all by itself under a variety of not-so-well documented >> circumstances. > > If that is true, then the code today is broken as well, when the flag has > been cleared and both prev and next have the flag set. Then it won't be > updated for the next task. > > The we should not use the TIF flag and store a debugmask in thread info and > do: > > if (prev->debugmask || next->debugmask) { > if (static_cpu_has(X86_FEATURE_BLOCKSTEP)) { > rdmsrl(MSR_IA32_DEBUGCTLMSR, msrval); > msrval &= DEBUGCTLMSR_BTF; > msrval |= next->debugmask; > } > } Seems reasonable to me. Although keeping it in flags might simplify the logic a bit. FWIW, I doubt we care about performance much when either prev or next has the bit set. --Andy