From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3rZXyL2MNFzDqRd for ; Thu, 23 Jun 2016 04:22:50 +1000 (AEST) Date: Wed, 22 Jun 2016 13:22:45 -0500 From: Josh Poimboeuf To: Andy Lutomirski Cc: Jiri Kosina , Ingo Molnar , X86 ML , Heiko Carstens , "linux-s390@vger.kernel.org" , live-patching@vger.kernel.org, Michael Ellerman , Chris J Arges , Jessica Yu , linuxppc-dev@lists.ozlabs.org, Petr Mladek , Jiri Slaby , "linux-kernel@vger.kernel.org" , Vojtech Pavlik , Miroslav Benes , Peter Zijlstra Subject: Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Message-ID: <20160622182245.4u72jgjg2khxracz@treble> References: <20160429224112.kl3jlk7ccvfceg2r@treble> <20160502135243.jkbnonaesv7zfios@treble> <20160519231546.yvtqz5wacxvykvn2@treble> <20160524022805.sazm456jorfqvdr7@treble> <20160622163011.4fjwalehrzk4a32t@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 In-Reply-To: List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Wed, Jun 22, 2016 at 10:59:23AM -0700, Andy Lutomirski wrote: > > So I got a chance to look at this some more. I'm thinking that to make > > this feature more consistently useful, we shouldn't only annotate > > pt_regs frames for calls to handlers; other calls should be annotated as > > well: preempt_schedule_irq, CALL_enter_from_user_mode, > > prepare_exit_to_usermode, SWAPGS, TRACE_IRQS_OFF, DISABLE_INTERRUPTS, > > etc. That way, the unwinder will always be able to find pt_regs from an > > interrupt/exception, even if starting from one of these other calls. > > > > But then, things get ugly. You have to either setup and tear down the > > frame for every possible call, or do a higher-level setup/teardown > > across multiple calls, which invalidates several assumptions in the > > entry code about the location of pt_regs on the stack. > > > > Also problematic is that several of the macros (like TRACE_IRQS_IRETQ) > > make assumptions about the location of pt_regs. And they're used by > > both syscall and interrupt code. So if we didn't create a frame pointer > > header for syscalls, we'd basically need two versions of the macros: one > > for irqs/exceptions and one for syscalls. > > > > So I think the cleanest way to handle this is to always allocate two > > extra registers on the stack in ALLOC_PT_GPREGS_ON_STACK. Then all > > entry code can assume that pt_regs is at a constant location, and all > > the above problems go away. Another benefit is that we'd only need two > > saves instead of three -- the pointer to pt_regs is no longer needed > > since pt_regs is always immediately after the frame header. > > > > I worked up a patch to implement this -- see below. It writes the frame > > pointer in all entry paths, including syscalls. This helps keep the > > code simple. > > > > The downside is a small performance penalty: with getppid()-in-a-loop on > > my laptop, the average syscall went from 52ns to 53ns, which is about a > > 2% slowdown. But I doubt it would be measurable in a real-world > > workload. > > > > It looks like about half the slowdown is due to the extra stack > > allocation (which presumably adds a little d-cache pressure on the stack > > memory) and the other half is due to the stack writes. > > > > I could remove the writes from the syscall path but it would only save > > about half a ns, and it would make the code less robust. Plus it's nice > > to have the consistency of having *all* pt_regs frames annotated. > > This is a bit messy, and I'm not really sure that the entry code > should be have to operate under constraints like this. Also, > convincing myself this works for NMI sounds unpleasant. > > Maybe we should go back to my idea of just listing the call sites in a table. So are you suggesting something like: .macro ENTRY_CALL func pt_regs_offset=0 call \func 1: .pushsection .entry_calls, "a" .long 1b - . .long \pt_regs_offset .popsection .endm and then change every call in the entry code to ENTRY_CALL? -- Josh