From patchwork Tue May 22 14:17:59 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Vorontsov X-Patchwork-Id: 8886 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 5BFEF23F0A for ; Tue, 22 May 2012 14:21:05 +0000 (UTC) Received: from mail-gg0-f180.google.com (mail-gg0-f180.google.com [209.85.161.180]) by fiordland.canonical.com (Postfix) with ESMTP id 2C7D9A18C3D for ; Tue, 22 May 2012 14:21:05 +0000 (UTC) Received: by mail-gg0-f180.google.com with SMTP id f1so6684516ggn.11 for ; Tue, 22 May 2012 07:21:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf:from:to:cc :subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=jBBYGYWnpHt5TCrkc/45dG/vE63CwlRPHipKozQljPw=; b=OdJrGHUtKzKmD6KDBe97bPPdFKwgYr/5cFO4tf1heoAYiRonCOC3OilN8y25OI5GC1 26XJB2pD6SzUrYb+J2kKI9H64e4Q9aaP5fu4kJdNRCuI1MxxYcRC4XOwkQ4L13fbrfA7 qOhgQ/sucrcWItoK7Qg7uEfq7jC2C3koahMcueBKQARDcL1XLvdeYfWynL2GTndDRSAg dTDl+JxxXT1iCm+FBxxUjHwK8pbKswadZD4NIu1zvoBHr5WuEsNdsZlmcRcZud/vq9cT 3M31vk3oH2FS59UyjVnWl2oD1i0OcSsgeatrIYxfxNc8/LoGog+bphVKdy5kbFx73x2R Zarg== Received: by 10.50.154.169 with SMTP id vp9mr9596180igb.53.1337696464640; Tue, 22 May 2012 07:21:04 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.35.72 with SMTP id o8csp369579ibd; Tue, 22 May 2012 07:21:04 -0700 (PDT) Received: by 10.50.47.230 with SMTP id g6mr9791714ign.74.1337696464186; Tue, 22 May 2012 07:21:04 -0700 (PDT) Received: from mail-pz0-f50.google.com (mail-pz0-f50.google.com [209.85.210.50]) by mx.google.com with ESMTPS id t9si31663461pbj.12.2012.05.22.07.21.03 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 22 May 2012 07:21:04 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.210.50 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) client-ip=209.85.210.50; Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.210.50 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) smtp.mail=anton.vorontsov@linaro.org Received: by mail-pz0-f50.google.com with SMTP id h15so9932040dan.37 for ; Tue, 22 May 2012 07:21:03 -0700 (PDT) Received: by 10.68.229.65 with SMTP id so1mr80734093pbc.2.1337696463802; Tue, 22 May 2012 07:21:03 -0700 (PDT) Received: from localhost (c-71-204-165-222.hsd1.ca.comcast.net. [71.204.165.222]) by mx.google.com with ESMTPS id nw10sm24789890pbb.20.2012.05.22.07.21.01 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 22 May 2012 07:21:02 -0700 (PDT) From: Anton Vorontsov To: Greg Kroah-Hartman , Kees Cook , Colin Cross , Tony Luck Cc: Arnd Bergmann , John Stultz , Shuah Khan , arve@android.com, Rebecca Schultz Zavin , Jesper Juhl , Randy Dunlap , Stephen Boyd , Thomas Meyer , Andrew Morton , Marco Stornelli , WANG Cong , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, linaro-kernel@lists.linaro.org, patches@linaro.org, kernel-team@android.com Subject: [PATCH 16/16] pstore/platform: Disable automatic updates by default Date: Tue, 22 May 2012 07:17:59 -0700 Message-Id: <1337696279-8994-16-git-send-email-anton.vorontsov@linaro.org> X-Mailer: git-send-email 1.7.9.2 In-Reply-To: <20120522141717.GA31574@lizard> References: <20120522141717.GA31574@lizard> X-Gm-Message-State: ALoCoQl7MaepupCce1mw8Qb+hqyfwYMB/8Dp0fBjp0Zfq4pkzK3D4rEyZpZnVSbFsFGEw+Gqq3Ni Having automatic updates seems pointless for production system, and even dangerous and thus counter-productive: 1. If we can mount pstore, or read files, we can as well read /proc/kmsg. So, there's little point in duplicating the functionality and present the same information but via another userland ABI; 2. Expecting the kernel to behave sanely after oops/panic is naive. It might work, but you'd rather not try it. Screwed up kernel can do rather bad things, like recursive faults[1]; and pstore rather provoking bad things to happen. It uses: 1. Timers (assumes sane interrupts state); 2. Workqueues and mutexes (assumes scheduler in a sane state); 3. kzalloc (a working slab allocator); That's too much for a dead kernel, so the debugging facility itself might just make debugging harder, which is not what we want. Maybe for non-oops message types it would make sense to re-enable automatic updates, but so far I don't see any use case for this. Even for tracing, it has its own run-time/normal ABI, so we're only interested in pstore upon next boot, to retrieve what has gone wrong with HW or SW. So, let's disable the updates by default. [1] BUG: unable to handle kernel paging request at fffffffffffffff8 IP: [] kthread_data+0xb/0x20 [...] Process kworker/0:1 (pid: 14, threadinfo ffff8800072c0000, task ffff88000725b100) [... Call Trace: [] wq_worker_sleeping+0x10/0xa0 [] __schedule+0x568/0x7d0 [] ? trace_hardirqs_on+0xd/0x10 [] ? call_rcu_sched+0x12/0x20 [] ? release_task+0x156/0x2d0 [] ? release_task+0x1e/0x2d0 [] ? trace_hardirqs_on+0xd/0x10 [] schedule+0x24/0x70 [] do_exit+0x1f8/0x370 [] oops_end+0x77/0xb0 [] no_context+0x1a6/0x1b5 [] __bad_area_nosemaphore+0x1ce/0x1ed [] ? ttwu_queue+0xc6/0xe0 [] bad_area_nosemaphore+0xe/0x10 [] do_page_fault+0x2c7/0x450 [] ? __lock_release+0x6b/0xe0 [] ? mark_held_locks+0x61/0x140 [] ? __wake_up+0x4e/0x70 [] ? trace_hardirqs_off_thunk+0x3a/0x3c [] ? pstore_register+0x120/0x120 [] page_fault+0x1f/0x30 [] ? pstore_register+0x120/0x120 [] ? memcpy+0x68/0x110 [] ? pstore_get_records+0x3a/0x130 [] ? persistent_ram_copy_old+0x64/0x90 [] ramoops_pstore_read+0x84/0x130 [] pstore_get_records+0x79/0x130 [] ? process_one_work+0x116/0x450 [] ? pstore_register+0x120/0x120 [] pstore_dowork+0xe/0x10 [] process_one_work+0x174/0x450 [] ? process_one_work+0x116/0x450 [] worker_thread+0x123/0x2d0 [] ? manage_workers.isra.28+0x120/0x120 [] kthread+0x8e/0xa0 [] kernel_thread_helper+0x4/0x10 [] ? retint_restore_args+0xe/0xe [] ? __init_kthread_worker+0x70/0x70 [] ? gs_change+0xb/0xb Code: be e2 00 00 00 48 c7 c7 d1 2a 4e 81 e8 bf fb fd ff 48 8b 5d f0 4c 8b 65 f8 c9 c3 0f 1f 44 00 00 48 8b 87 08 02 00 00 55 48 89 e5 <48> 8b 40 f8 5d c3 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 RIP [] kthread_data+0xb/0x20 RSP CR2: fffffffffffffff8 ---[ end trace 996a332dc399111d ]--- Fixing recursive fault but reboot is needed! Signed-off-by: Anton Vorontsov --- fs/pstore/platform.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 4f49bb4..1dbf49d 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -41,10 +41,11 @@ * whether the system is actually still running well enough * to let someone see the entry */ -static int pstore_update_ms = 60000; +static int pstore_update_ms = -1; module_param_named(update_ms, pstore_update_ms, int, 0600); MODULE_PARM_DESC(update_ms, "milliseconds before pstore updates its content " - "(default is 60000; -1 means runtime updates are disabled)"); + "(default is -1, which means runtime updates are disabled; " + "enabling this option is not safe)"); static int pstore_new_entry;