From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755766AbbHYW0a (ORCPT ); Tue, 25 Aug 2015 18:26:30 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:60577 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751798AbbHYW02 (ORCPT ); Tue, 25 Aug 2015 18:26:28 -0400 Date: Tue, 25 Aug 2015 15:26:26 -0700 From: Stephen Boyd To: Douglas Anderson Cc: Kees Cook , Nicolas Pitre , Aapo Vienamo , linux@arm.linux.org.uk, masami.hiramatsu.pt@hitachi.com, wangnan0@huawei.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] arm: kgdb: Don't try to stop the machine when setting breakpoints Message-ID: <20150825222626.GK19120@codeaurora.org> References: <1440540165-28875-1-git-send-email-dianders@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1440540165-28875-1-git-send-email-dianders@chromium.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/25, Douglas Anderson wrote: > In (23a4e40 arm: kgdb: Handle read-only text / modules) we moved to > using patch_text() to set breakpoints so that we could handle the case > when we had CONFIG_DEBUG_RODATA. That patch used patch_text(). > Unfortunately, patch_text() assumes that we're not in atomic context > when it runs since it needs to grab a mutex and also wait for other > CPUs to stop (which it does with a completion). > > This would result in a stack crawl if you had > CONFIG_DEBUG_ATOMIC_SLEEP and tried to set a breakpoint in kgdb. The > crawl looked something like: > > BUG: scheduling while atomic: swapper/0/0/0x00010007 > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc7-00133-geb63b34 #1073 > Hardware name: Rockchip (Device Tree) > (unwind_backtrace) from [] (show_stack+0x20/0x24) > (show_stack) from [] (dump_stack+0x84/0xb8) > (dump_stack) from [] (__schedule_bug+0x54/0x6c) > (__schedule_bug) from [] (__schedule+0x80/0x668) > (__schedule) from [] (schedule+0xb8/0xd4) > (schedule) from [] (schedule_timeout+0x2c/0x234) > (schedule_timeout) from [] (wait_for_common+0xf4/0x188) > (wait_for_common) from [] (wait_for_completion+0x20/0x24) > (wait_for_completion) from [] (__stop_cpus+0x58/0x70) > (__stop_cpus) from [] (stop_cpus+0x3c/0x54) > (stop_cpus) from [] (__stop_machine+0xcc/0xe8) > (__stop_machine) from [] (stop_machine+0x34/0x44) > (stop_machine) from [] (patch_text+0x28/0x34) > (patch_text) from [] (kgdb_arch_set_breakpoint+0x40/0x4c) > (kgdb_arch_set_breakpoint) from [] (kgdb_validate_break_address+0x2c/0x60) > (kgdb_validate_break_address) from [] (dbg_set_sw_break+0x1c/0xdc) > (dbg_set_sw_break) from [] (gdb_serial_stub+0x9c4/0xba4) > (gdb_serial_stub) from [] (kgdb_cpu_enter+0x1f8/0x60c) > (kgdb_cpu_enter) from [] (kgdb_handle_exception+0x19c/0x1d0) > (kgdb_handle_exception) from [] (kgdb_compiled_brk_fn+0x30/0x3c) > (kgdb_compiled_brk_fn) from [] (do_undefinstr+0x1a4/0x20c) > (do_undefinstr) from [] (__und_svc_finish+0x0/0x34) > > It turns out that when we're in kgdb all the CPUs are stopped anyway > so there's no reason we should be calling patch_text(). We can > instead directly call __patch_text() which assumes that CPUs have > already been stopped. > > Fixes: 23a4e4050ba9 ("arm: kgdb: Handle read-only text / modules") > Reported-by: Aapo Vienamo > Signed-off-by: Douglas Anderson > --- Reviewed-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project