From: Vineet Gupta <Vineet.Gupta1@synopsys.com> To: Al Viro <viro@ZenIV.linux.org.uk> Cc: <linux-arch@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <tglx@linutronix.de>, <arnd@arndb.de> Subject: Re: [RFC PATCH v1 30/31] ARC: switch to generic kernel_execve() and sys_execve() Date: Sat, 17 Nov 2012 19:31:59 +0530 [thread overview] Message-ID: <50A798D7.9040306@synopsys.com> (raw) In-Reply-To: <20121116040847.GA22671@ZenIV.linux.org.uk> On Friday 16 November 2012 09:38 AM, Al Viro wrote: > On Wed, Nov 07, 2012 at 10:47:53AM +0100, Vineet Gupta wrote: >> +; When we land here, pt_regs have already been updated in-place correctly >> +; A pointer to them is also passed by kernel_execve, we just need to make sure >> +; that SP is set to point to them. >> +ARC_ENTRY ret_from_kernel_execve >> + ; Force SP to "normal" pt_regs just populated. >> + b.d ret_from_system_call >> + mov sp, r0 > won't that splatter crap into regs->r0? IOW, why not branch to > ret_from_exception here? Yes it does - although I didn't notice the ill-effect since execve as an API doesn't expect r0 to have a certain value (like fork for child). Fixed in next revision. > >> +ARC_EXIT ret_from_kernel_execve > Another thing: why not fold that branch to ret_from_exception into the end of > ret_from_kernel_thread() (instead of calling sys_exit()), select > GENERIC_KERNEL_EXECVE and lose __ARCH_WANT_KERNEL_EXECVE. If you noticed I only had 3 patches and didn't have the equivalent of "switch to saner kernel_execve() semantics" which would achieve above because the bug you elucidate to below was giving me grief in bring up (and I couldn't spend enough time debugging it). > Actually, now that I look at your ret_from_kernel_thread... How the hell > will it cope with kernel_thread() payload trying to return? AFAICS, this > j.d [r1] will lose the return address, won't it? Exactly so. I was missing the equivalent of following ARC_ENTRY ret_from_kernel_thread bl @schedule_tail ld r1, [sp, PT_r1] - j.d [r1] + jl.d [r1] ld r0, [sp, PT_r0] j @sys_exit ARC_EXIT ret_from_kernel_thread And since none of the bootup kernel threads took the return path I had a working system - the problem only showed up when kernel_execve( ) started returning to exercise the broken path above. > And while we are at it, > I would suggest passing callback and its argument via callee-saved registers - > makes for simpler life in ret_from_kernel_thread(), since switch_to() itself > will take care of loading those... Absolutely - done that too ! Would you prefer to take a look at the updated patches now or will you rather wait for v2 series which might take a week or two. Thanks for taking the time to review this stuff. -Vineet
WARNING: multiple messages have this Message-ID (diff)
From: Vineet Gupta <Vineet.Gupta1@synopsys.com> To: Al Viro <viro@ZenIV.linux.org.uk> Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, arnd@arndb.de Subject: Re: [RFC PATCH v1 30/31] ARC: switch to generic kernel_execve() and sys_execve() Date: Sat, 17 Nov 2012 19:31:59 +0530 [thread overview] Message-ID: <50A798D7.9040306@synopsys.com> (raw) In-Reply-To: <20121116040847.GA22671@ZenIV.linux.org.uk> On Friday 16 November 2012 09:38 AM, Al Viro wrote: > On Wed, Nov 07, 2012 at 10:47:53AM +0100, Vineet Gupta wrote: >> +; When we land here, pt_regs have already been updated in-place correctly >> +; A pointer to them is also passed by kernel_execve, we just need to make sure >> +; that SP is set to point to them. >> +ARC_ENTRY ret_from_kernel_execve >> + ; Force SP to "normal" pt_regs just populated. >> + b.d ret_from_system_call >> + mov sp, r0 > won't that splatter crap into regs->r0? IOW, why not branch to > ret_from_exception here? Yes it does - although I didn't notice the ill-effect since execve as an API doesn't expect r0 to have a certain value (like fork for child). Fixed in next revision. > >> +ARC_EXIT ret_from_kernel_execve > Another thing: why not fold that branch to ret_from_exception into the end of > ret_from_kernel_thread() (instead of calling sys_exit()), select > GENERIC_KERNEL_EXECVE and lose __ARCH_WANT_KERNEL_EXECVE. If you noticed I only had 3 patches and didn't have the equivalent of "switch to saner kernel_execve() semantics" which would achieve above because the bug you elucidate to below was giving me grief in bring up (and I couldn't spend enough time debugging it). > Actually, now that I look at your ret_from_kernel_thread... How the hell > will it cope with kernel_thread() payload trying to return? AFAICS, this > j.d [r1] will lose the return address, won't it? Exactly so. I was missing the equivalent of following ARC_ENTRY ret_from_kernel_thread bl @schedule_tail ld r1, [sp, PT_r1] - j.d [r1] + jl.d [r1] ld r0, [sp, PT_r0] j @sys_exit ARC_EXIT ret_from_kernel_thread And since none of the bootup kernel threads took the return path I had a working system - the problem only showed up when kernel_execve( ) started returning to exercise the broken path above. > And while we are at it, > I would suggest passing callback and its argument via callee-saved registers - > makes for simpler life in ret_from_kernel_thread(), since switch_to() itself > will take care of loading those... Absolutely - done that too ! Would you prefer to take a look at the updated patches now or will you rather wait for v2 series which might take a week or two. Thanks for taking the time to review this stuff. -Vineet
next prev parent reply other threads:[~2012-11-17 14:05 UTC|newest] Thread overview: 141+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-11-07 9:47 [RFC Patch v1 00/31] Synopsys ARC Linux kernel Port Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 01/31] ARC: Generic Headers Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 02/31] ARC: irqflags Vineet Gupta 2012-11-12 19:50 ` Thomas Gleixner 2013-01-01 7:44 ` Vineet Gupta 2013-01-01 7:44 ` Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 03/31] ARC: atomic/bitops/cmpxchg/barriers Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 04/31] asm-generic headers: uaccess.h to conditionally define segment_eq() Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 05/31] ARC: uaccess friends Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 06/31] asm-generic headers: Allow yet more arch overrides in checksum.h Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 07/31] ARC: checksum/byteorder/swab routines Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 08/31] ARC: Fundamental ARCH data-types/defines Vineet Gupta 2012-11-08 7:10 ` Jonas Bonn 2012-11-08 18:52 ` Vineet Gupta 2012-11-08 20:36 ` Jonas Bonn 2012-11-12 13:58 ` Vineet Gupta 2012-11-12 14:12 ` Arnd Bergmann 2012-11-07 9:47 ` [RFC PATCH v1 09/31] ARC: spinlock/rwlock/mutex primitives Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 10/31] ARC: string library Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 11/31] ARC: Low level IRQ/Trap/Exception(non-MMU) Handling Vineet Gupta 2012-11-16 4:58 ` Al Viro 2012-12-27 9:00 ` Vineet Gupta 2012-12-27 9:00 ` Vineet Gupta 2012-12-27 13:29 ` Vineet Gupta 2012-12-27 13:29 ` Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 12/31] ARC: Interrupt Handling Vineet Gupta 2012-11-12 20:08 ` Thomas Gleixner 2013-01-01 10:46 ` Vineet Gupta 2013-01-01 10:46 ` Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 13/31] ARC: Non-MMU Exception Handling Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 14/31] ARC: syscall support Vineet Gupta 2012-11-07 14:21 ` Arnd Bergmann 2012-11-09 9:50 ` James Hogan 2012-11-09 9:50 ` James Hogan 2012-11-13 11:41 ` James Hogan 2012-11-13 11:41 ` James Hogan 2012-11-13 12:01 ` Jonas Bonn 2012-11-13 12:11 ` James Hogan 2012-11-13 12:11 ` James Hogan 2012-11-14 12:23 ` Arnd Bergmann 2012-11-14 12:31 ` James Hogan 2012-11-14 12:31 ` James Hogan 2012-11-13 10:13 ` Gilad Ben-Yossef 2012-11-13 10:37 ` Arnd Bergmann 2012-11-15 6:15 ` Vineet Gupta 2012-11-15 6:15 ` Vineet Gupta 2012-11-15 12:35 ` Arnd Bergmann 2013-01-17 5:13 ` Vineet Gupta 2013-01-17 5:13 ` Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 15/31] ARC: Process/scheduling/clock/Timers/Delay Management Vineet Gupta 2012-11-12 20:29 ` Thomas Gleixner 2013-01-02 7:13 ` Vineet Gupta 2013-01-02 7:13 ` Vineet Gupta 2013-01-02 8:45 ` Vineet Gupta 2013-01-02 8:45 ` Vineet Gupta 2013-01-04 13:01 ` Frederic Weisbecker 2012-11-07 9:47 ` [RFC PATCH v1 16/31] ARC: Signal handling Vineet Gupta 2012-11-16 5:26 ` Al Viro 2012-12-28 12:34 ` Vineet Gupta 2012-12-28 12:34 ` Vineet Gupta 2012-12-28 12:42 ` [PATCH 1/2] ARC: [Review] Preparing to fix incorrect syscall restarts due to signals Vineet Gupta 2012-12-28 12:42 ` Vineet Gupta 2012-12-28 12:42 ` [PATCH 2/2] ARC: [Review] Prevent incorrect syscall restarts Vineet Gupta 2012-12-28 12:42 ` Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 17/31] ARC: Cache Flush Management Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 18/31] ARC: Page Table Management Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 19/31] ARC: MMU Context Management Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 20/31] ARC: MMU Exception Handling Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 21/31] ARC: TLB flush Handling Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 22/31] ARC: Page Fault handling (incl uaccess fixup) Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 23/31] ARC: I/O and DMA Mappings Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 24/31] ARC: startup #1: low-level, setup_arch(), /proc/cpuinfo, mem init Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 25/31] ARC: [plat-arcfpga] Hooking up platform to ARC UART Vineet Gupta 2012-11-07 14:16 ` Arnd Bergmann 2013-01-07 13:10 ` Vineet Gupta 2013-01-07 13:10 ` Vineet Gupta 2013-01-07 13:46 ` Arnd Bergmann 2013-01-07 14:04 ` Vineet Gupta 2013-01-07 14:04 ` Vineet Gupta 2013-01-07 14:36 ` Arnd Bergmann 2013-01-14 7:35 ` early init dt for earlyprintk (was Re: [RFC PATCH v1 25/31] ARC: [plat-arcfpga] Hooking up platform to ARC UART) Vineet Gupta 2013-01-14 7:35 ` Vineet Gupta 2013-01-14 9:48 ` James Hogan 2013-01-14 9:48 ` James Hogan 2013-01-14 10:09 ` Vineet Gupta 2013-01-14 10:09 ` Vineet Gupta 2013-01-14 10:54 ` Arnd Bergmann 2013-01-17 7:29 ` [RFC PATCH v1 25/31] ARC: [plat-arcfpga] Hooking up platform to ARC UART Vineet Gupta 2013-01-17 7:29 ` Vineet Gupta 2013-01-17 10:52 ` Arnd Bergmann 2012-11-07 9:47 ` [RFC PATCH v1 26/31] ARC: Build system: Makefiles, Kconfig, Linker script Vineet Gupta 2012-11-07 14:13 ` Arnd Bergmann 2013-01-02 14:30 ` Vineet Gupta 2013-01-02 14:48 ` Arnd Bergmann 2013-01-03 7:58 ` Vineet Gupta 2013-01-03 7:58 ` Vineet Gupta 2013-01-03 8:25 ` Arnd Bergmann 2013-03-11 12:29 ` SYSV IPC broken for no-legacy syscall kernels (was Re: [RFC PATCH v1 26/31] ARC: Build system: Makefiles, Kconfig, Linker script) Vineet Gupta 2013-03-11 12:29 ` Vineet Gupta 2013-03-11 12:44 ` James Hogan 2013-03-11 12:44 ` James Hogan 2013-03-11 12:56 ` Vineet Gupta 2013-03-11 12:56 ` Vineet Gupta 2013-03-11 13:07 ` James Hogan 2013-03-11 13:07 ` James Hogan 2013-03-11 13:30 ` Arnd Bergmann 2013-03-11 13:48 ` Vineet Gupta 2013-03-11 13:48 ` Vineet Gupta 2013-03-11 14:50 ` Arnd Bergmann 2012-11-15 17:49 ` [RFC PATCH v1 26/31] ARC: Build system: Makefiles, Kconfig, Linker script James Hogan 2012-11-15 17:49 ` James Hogan 2012-11-15 19:30 ` Ralf Baechle 2012-11-16 6:36 ` Vineet Gupta 2012-11-16 6:36 ` Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 27/31] ARC: Last bits (stubs) to get to a running kernel with UART Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 28/31] ARC: split ret_from_fork, simplify kernel_thread() Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 29/31] ARC: switch to generic kernel_thread() Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 30/31] ARC: switch to generic kernel_execve() and sys_execve() Vineet Gupta 2012-11-16 4:08 ` Al Viro 2012-11-17 14:01 ` Vineet Gupta [this message] 2012-11-17 14:01 ` Vineet Gupta 2012-11-07 9:47 ` [RFC PATCH v1 31/31] ARC: [plat-arcfpga] defconfig Vineet Gupta 2012-11-07 14:06 ` Arnd Bergmann 2012-11-12 14:18 ` James Hogan 2012-11-12 14:18 ` James Hogan 2012-11-12 14:21 ` Arnd Bergmann 2012-11-07 14:36 ` [RFC Patch v1 00/31] Synopsys ARC Linux kernel Port Arnd Bergmann 2012-11-08 19:09 ` Vineet Gupta 2012-11-07 20:46 ` Gilad Ben-Yossef 2012-11-20 13:47 ` Pavel Machek 2012-11-20 13:49 ` Vineet Gupta 2012-11-20 13:49 ` Vineet Gupta 2012-11-20 13:59 ` Pavel Machek 2012-11-20 14:17 ` Vineet Gupta 2012-11-20 14:17 ` Vineet Gupta 2013-01-18 19:46 ` Pavel Machek 2013-01-18 22:17 ` Arnd Bergmann 2013-01-19 10:15 ` Pavel Machek 2013-01-19 12:32 ` Vineet Gupta 2013-01-19 12:32 ` Vineet Gupta 2013-01-19 17:02 ` Pavel Machek
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=50A798D7.9040306@synopsys.com \ --to=vineet.gupta1@synopsys.com \ --cc=arnd@arndb.de \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=viro@ZenIV.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.