Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755147AbbGQVX6 (ORCPT ); Fri, 17 Jul 2015 17:23:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44887 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754031AbbGQVX4 (ORCPT ); Fri, 17 Jul 2015 17:23:56 -0400 Date: Fri, 17 Jul 2015 16:23:54 -0500 From: Josh Poimboeuf To: Andy Lutomirski Cc: Peter Zijlstra , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Michal Marek , Andy Lutomirski , Borislav Petkov , Linus Torvalds , Andi Kleen , Pedro Alves , X86 ML , live-patching@vger.kernel.org, "linux-kernel@vger.kernel.org" Subject: Re: [RFC PATCH 12/21] sched: Add __schedule() to stackvalidate whitelist Message-ID: <20150717212354.GF12761@treble.redhat.com> References: <6decd725802447eb2ba0c914c3d70610a606c2b7.1437150175.git.jpoimboe@redhat.com> <20150717194613.GQ19282@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2297 Lines: 57 On Fri, Jul 17, 2015 at 12:58:12PM -0700, Andy Lutomirski wrote: > On Fri, Jul 17, 2015 at 12:46 PM, Peter Zijlstra wrote: > > On Fri, Jul 17, 2015 at 11:47:28AM -0500, Josh Poimboeuf wrote: > >> stackvalidate reports the following warnings for __schedule(): > >> > >> stackvalidate: kernel/sched/core.o: __schedule()+0x3e7: duplicate frame pointer save > >> stackvalidate: kernel/sched/core.o: __schedule()+0x424: sibling call from callable instruction with changed frame pointer > >> stackvalidate: kernel/sched/core.o: __schedule()+0x431: call without frame pointer save/setup > >> stackvalidate: kernel/sched/core.o: __schedule()+0x8b8: frame pointer state mismatch > >> stackvalidate: kernel/sched/core.o: __schedule()+0x447: frame pointer state mismatch > >> > >> __schedule() is obviously a special case which is allowed to do unusual > >> things with the frame pointer. > > > > Yes, but is the code actually correct? We can't dismiss the warnings > > just on that basis alone. > > It's really only __switch_to that does weird things, right? I kinda > want to rework that thing anyway to have a well-defined saved state > format anyway, which would have the nice benefit of letting us get rid > of all the ret_from_fork crap. > > That is, we'd context switch like: > > static inline void __switch_to(...) { > switch extra stuff; > switch everything except gpregs; > asm volatile ("call __switch_stack_and_ip" : [__sp thing goes here] > : "S" (&prev->bottom_of_stack), "D" (&next->bottom_of_stack) : > "basically all regs and flags"); > } > > Then the low level bit would be: > > __switch_stack_and_ip: > pushq %rbp > mov %rsp, (%rsi) > mov (%rdi), %rsp > popq %rbp > ret > > Now, when we create a new task, we can set up its stack directly > rather than setting some TI flag, because we actually know the layout. > > Hmm? I think that would make stackvalidate much happier. I'll drop this patch for now so stackvalidate will keep reminding me to bug you to implement this ;-) -- Josh -- 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/