From patchwork Sat May 26 13:20:29 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Vorontsov X-Patchwork-Id: 9001 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 49DB723EB5 for ; Sat, 26 May 2012 13:22:55 +0000 (UTC) Received: from mail-gh0-f180.google.com (mail-gh0-f180.google.com [209.85.160.180]) by fiordland.canonical.com (Postfix) with ESMTP id 1A06BA18A3B for ; Sat, 26 May 2012 13:22:55 +0000 (UTC) Received: by mail-gh0-f180.google.com with SMTP id z12so1021069ghb.11 for ; Sat, 26 May 2012 06:22:54 -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=ini+Oe2C4NnFKyhghld9R0OquJQy0iW0sdk9rK0rebo=; b=f1PQBmmSs8uxqie0j50L6hqxoLiR7Ch6HJ3cOLXyWf6jv/YaHvXYPM6EP2aTxz1P0z wweNRvX7FFtv4Axz9gyz96MO0XjXnhAVpbHUnvznjTvO1QmGWbTvN74AXoCxwWwobQ/b iDcOPkiSLU0MWXzy+F9yR7DStiNOxUebNoT+lzkIiUxOlQBZOTAf7+s1//hkbhDTexJ1 P/xB8v2Mf5NhsURHF58bneQDll+3+9HV8c74W8+3Zr9bfCYJ8UhsrYG8Z/N6LyMroqpt Ht4lAsYnuIeCQNzdMcAmSKu2/AvSL0xq6Tz+k8ehyfYPZmXrOAepSqo8pqIC7ksMTLNo 8CUA== Received: by 10.42.89.72 with SMTP id f8mr1243696icm.33.1338038574567; Sat, 26 May 2012 06:22:54 -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.24.148 with SMTP id v20csp86353ibb; Sat, 26 May 2012 06:22:54 -0700 (PDT) Received: by 10.68.138.166 with SMTP id qr6mr7450802pbb.163.1338038573840; Sat, 26 May 2012 06:22:53 -0700 (PDT) Received: from mail-pb0-f50.google.com (mail-pb0-f50.google.com [209.85.160.50]) by mx.google.com with ESMTPS id ru5si9888927pbc.11.2012.05.26.06.22.53 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 26 May 2012 06:22:53 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.160.50 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) client-ip=209.85.160.50; Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.160.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-pb0-f50.google.com with SMTP id rr4so3293117pbb.37 for ; Sat, 26 May 2012 06:22:53 -0700 (PDT) Received: by 10.68.224.70 with SMTP id ra6mr7922954pbc.11.1338038573523; Sat, 26 May 2012 06:22:53 -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 pe2sm12524528pbc.59.2012.05.26.06.22.50 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 26 May 2012 06:22:52 -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 11/11] pstore/platform: Disable automatic updates by default Date: Sat, 26 May 2012 06:20:29 -0700 Message-Id: <1338038429-21945-11-git-send-email-anton.vorontsov@linaro.org> X-Mailer: git-send-email 1.7.9.2 In-Reply-To: <20120526131747.GA15054@lizard> References: <20120526131747.GA15054@lizard> X-Gm-Message-State: ALoCoQnDIKPL5D0x9ot4GbhwdrdSfBv1wFZBss+FcVRg44JP59bppv6v8NgNGqaM7Wk2YcNquLsp 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 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 34ca314..be4614f 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -41,10 +41,12 @@ * 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, it may lead to further " + "corruption on Oopses)"); static int pstore_new_entry;