From 9f244e9cfd70c7c0f82d3c92ce772ab2a92d9f64 Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Fri, 11 Jan 2013 18:09:41 +0000 Subject: pstore: Avoid deadlock in panic and emergency-restart path [Issue] When pstore is in panic and emergency-restart paths, it may be blocked in those paths because it simply takes spin_lock. This is an example scenario which pstore may hang up in a panic path: - cpuA grabs psinfo->buf_lock - cpuB panics and calls smp_send_stop - smp_send_stop sends IRQ to cpuA - after 1 second, cpuB gives up on cpuA and sends an NMI instead - cpuA is now in an NMI handler while still holding buf_lock - cpuB is deadlocked This case may happen if a firmware has a bug and cpuA is stuck talking with it more than one second. Also, this is a similar scenario in an emergency-restart path: - cpuA grabs psinfo->buf_lock and stucks in a firmware - cpuB kicks emergency-restart via either sysrq-b or hangcheck timer. And then, cpuB is deadlocked by taking psinfo->buf_lock again. [Solution] This patch avoids the deadlocking issues in both panic and emergency_restart paths by introducing a function, is_non_blocking_path(), to check if a cpu can be blocked in current path. With this patch, pstore is not blocked even if another cpu has taken a spin_lock, in those paths by changing from spin_lock_irqsave to spin_trylock_irqsave. In addition, according to a comment of emergency_restart() in kernel/sys.c, spin_lock shouldn't be taken in an emergency_restart path to avoid deadlock. This patch fits the comment below. /** * emergency_restart - reboot the system * * Without shutting down any hardware or taking any locks * reboot the system. This is called when we know we are in * trouble so this is our best effort to reboot. This is * safe to call in interrupt context. */ void emergency_restart(void) Signed-off-by: Seiji Aguchi Acked-by: Don Zickus Signed-off-by: Tony Luck --- include/linux/pstore.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'include') diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 1788909d9a99..75d01760c911 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -68,12 +68,18 @@ struct pstore_info { #ifdef CONFIG_PSTORE extern int pstore_register(struct pstore_info *); +extern bool pstore_cannot_block_path(enum kmsg_dump_reason reason); #else static inline int pstore_register(struct pstore_info *psi) { return -ENODEV; } +static inline bool +pstore_cannot_block_path(enum kmsg_dump_reason reason) +{ + return false; +} #endif #endif /*_LINUX_PSTORE_H*/ -- cgit v1.2.3 From a93bc0c6e07ed9bac44700280e65e2945d864fd4 Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Tue, 12 Feb 2013 13:04:41 -0800 Subject: efi_pstore: Introducing workqueue updating sysfs [Problem] efi_pstore creates sysfs entries, which enable users to access to NVRAM, in a write callback. If a kernel panic happens in an interrupt context, it may fail because it could sleep due to dynamic memory allocations during creating sysfs entries. [Patch Description] This patch removes sysfs operations from a write callback by introducing a workqueue updating sysfs entries which is scheduled after the write callback is called. Also, the workqueue is kicked in a just oops case. A system will go down in other cases such as panic, clean shutdown and emergency restart. And we don't need to create sysfs entries because there is no chance for users to access to them. efi_pstore will be robust against a kernel panic in an interrupt context with this patch. Signed-off-by: Seiji Aguchi Acked-by: Matt Fleming Signed-off-by: Tony Luck --- include/linux/efi.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/linux/efi.h b/include/linux/efi.h index 8b84916dc671..d0c68aee0925 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -728,7 +728,8 @@ struct efivars { * 1) ->list - adds, removals, reads, writes * 2) ops.[gs]et_variable() calls. * It must not be held when creating sysfs entries or calling kmalloc. - * ops.get_next_variable() is only called from register_efivars(), + * ops.get_next_variable() is only called from register_efivars() + * or efivar_update_sysfs_entries(), * which is protected by the BKL, so that path is safe. */ spinlock_t lock; -- cgit v1.2.3