Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755938Ab2HQTnZ (ORCPT ); Fri, 17 Aug 2012 15:43:25 -0400 Received: from usindpps03.hds.com ([207.126.252.16]:36846 "EHLO usindpps03.hds.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752184Ab2HQTnQ convert rfc822-to-8bit (ORCPT ); Fri, 17 Aug 2012 15:43:16 -0400 From: Seiji Aguchi To: "linux-kernel@vger.kernel.org" , "Luck, Tony (tony.luck@intel.com)" , "mikew@google.com" , "Matthew Garrett (mjg@redhat.com)" , "dzickus@redhat.com" CC: "dle-develop@lists.sourceforge.net" , Satoru Moriya Subject: [RFC][PATCH 2/2] efi_pstore: Introducing workqueue updating sysfs entries Thread-Topic: [RFC][PATCH 2/2] efi_pstore: Introducing workqueue updating sysfs entries Thread-Index: Ac18sHkwmdnRbMTcT9KbT5zgh21KoA== Date: Fri, 17 Aug 2012 19:43:04 +0000 Message-ID: Accept-Language: ja-JP, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.74.43.113] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.7.7855,1.0.260,0.0.0000 definitions=2012-08-17_03:2012-08-17,2012-08-17,1970-01-01 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_policy_notspam policy=outbound_policy score=0 spamscore=0 ipscore=0 suspectscore=1 phishscore=0 bulkscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=6.0.2-1203120001 definitions=main-1208170207 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6704 Lines: 220 [Problem] efi_pstore creates sysfs entries ,which enable users to access to NVRAM, in a write callback. If a kernel panic happens in interrupt contexts, pstore may fail because it could sleep due to dynamic memory allocations during creating sysfs entries. [Patch Description] This patch removes sysfs operations from write callback by introducing a workqueue updating sysfs entries which is kicked after a write callback of efi_pstore is called. efi_pstore will be robust against a kernel panic in an interrupt context with it. The workqueue works as follows. - A timer is registered during an initialization of efivars module. - A flag, update_sysfs_entries, is checked periodically with the timer. - The flag is set in a write callback, efi_pstore_write(). - Operation updating sysfs entries is kicked if the flag is set. Any operations ,like registering a timer, should not be added in a write callback because it may run in panic case and fail due to operations related to workqueue. To make efi_pstore work in panic case, a write callback should just log to NVRAM. Signed-off-by: Seiji Aguchi --- drivers/firmware/efivars.c | 127 ++++++++++++++++++++++++++++++++++++++++---- include/linux/efi.h | 3 +- 2 files changed, 118 insertions(+), 12 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index b6a813e..9f27ef6 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -146,6 +146,17 @@ efivar_create_sysfs_entry(struct efivars *efivars, efi_char16_t *variable_name, efi_guid_t *vendor_guid); +/* + * Prototype for workqueue functions updating sysfs entry + */ + +static void efivar_update_sysfs_entry(struct work_struct *); +#define EFIVAR_INTERVAL (60 * HZ) +static void efivar_timefunc(unsigned long); +static DEFINE_TIMER(efivar_timer, efivar_timefunc, 0, 0); +static DECLARE_WORK(efivar_work, efivar_update_sysfs_entry); +static int update_sysfs_entries; + /* Return the number of unicode characters in data */ static unsigned long utf16_strnlen(efi_char16_t *s, size_t maxlength) @@ -739,9 +750,6 @@ static int efi_pstore_write(enum pstore_type_id type, 0, NULL); } - if (found) - list_del(&found->list); - for (i = 0; i < DUMP_NAME_LEN; i++) efi_name[i] = name[i]; @@ -750,14 +758,7 @@ static int efi_pstore_write(enum pstore_type_id type, spin_unlock_irqrestore(&efivars->lock, flags); - if (found) - efivar_unregister(found); - - if (size) - ret = efivar_create_sysfs_entry(efivars, - utf16_strsize(efi_name, - DUMP_NAME_LEN * 2), - efi_name, &vendor); + update_sysfs_entries = 1; *id = part; return ret; @@ -1201,6 +1202,8 @@ int register_efivars(struct efivars *efivars, pstore_register(&efivars->efi_pstore_info); } + add_timer(&efivar_timer); + out: kfree(variable_name); @@ -1211,6 +1214,107 @@ EXPORT_SYMBOL_GPL(register_efivars); static struct efivars __efivars; static struct efivar_operations ops; +static void delete_sysfs_entry(void) +{ + struct efivars *efivars = &__efivars; + struct efivar_entry *entry, *n; + efi_status_t status; + unsigned long flags; + + list_for_each_entry_safe(entry, n, &efivars->list, list) { + spin_lock_irqsave(&efivars->lock, flags); + status = get_var_data_locked(efivars, &entry->var); + if (status != EFI_SUCCESS) { + list_del(&entry->list); + spin_unlock_irqrestore(&efivars->lock, flags); + efivar_unregister(entry); + } else + spin_unlock_irqrestore(&efivars->lock, flags); + } +} + +static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t *vendor) +{ + struct efivar_entry *entry, *n; + struct efivars *efivars = &__efivars; + unsigned long strsize1, strsize2; + bool found = false; + + strsize1 = utf16_strsize(variable_name, 1024); + list_for_each_entry_safe(entry, n, &efivars->list, list) { + strsize2 = utf16_strsize(entry->var.VariableName, 1024); + if (strsize1 == strsize2 && + !memcmp(variable_name, &(entry->var.VariableName), + strsize2) && + !efi_guidcmp(entry->var.VendorGuid, + *vendor)) { + found = true; + break; + } + } + return found; +} + +static void efivar_update_sysfs_entry(struct work_struct *work) +{ + struct efivars *efivars = &__efivars; + efi_guid_t vendor; + efi_char16_t *variable_name; + unsigned long variable_name_size = 1024, flags; + efi_status_t status = EFI_NOT_FOUND; + bool found; + + /* Delete unavailable sysfs entries */ + delete_sysfs_entry(); + + /* Add new sysfs entries */ + while (1) { + variable_name = kzalloc(variable_name_size, GFP_KERNEL); + if (!variable_name) { + pr_err("efivars: Memory allocation failed.\n"); + return; + } + + spin_lock_irqsave(&efivars->lock, flags); + found = false; + while (1) { + variable_name_size = 1024; + status = efivars->ops->get_next_variable( + &variable_name_size, + variable_name, + &vendor); + if (status != EFI_SUCCESS) { + break; + } else { + if (!variable_is_present(variable_name, + &vendor)) { + found = true; + break; + } + } + } + spin_unlock_irqrestore(&efivars->lock, flags); + + if (!found) { + kfree(variable_name); + break; + } else + efivar_create_sysfs_entry(efivars, + variable_name_size, + variable_name, &vendor); + } +} + +static void efivar_timefunc(unsigned long dummy) +{ + if (update_sysfs_entries) { + update_sysfs_entries = 0; + schedule_work(&efivar_work); + } + + mod_timer(&efivar_timer, jiffies + EFIVAR_INTERVAL); +} + /* * For now we register the efi subsystem with the firmware subsystem * and the vars subsystem with the efi subsystem. In the future, it @@ -1266,6 +1370,7 @@ static void __exit efivars_exit(void) { if (efi_enabled) { + del_timer_sync(&efivar_timer); unregister_efivars(&__efivars); kobject_put(efi_kobj); } diff --git a/include/linux/efi.h b/include/linux/efi.h index f1f5703..3e0c37a 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -641,7 +641,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_entry(), * which is protected by the BKL, so that path is safe. */ spinlock_t lock; -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/