Messages in this thread Patch in this message | | | From | Seiji Aguchi <> | Subject | [RFC][PATCH v2 2/3] efi_pstore: Introducing workqueue updating sysfs entries | Date | Mon, 20 Aug 2012 19:15:22 +0000 |
| |
[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 scheduled after a write callback of efi_pstore is called. efi_pstore will be robust against a kernel panic in an interrupt context with it.
Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com> --- drivers/firmware/efivars.c | 111 +++++++++++++++++++++++++++++++++++++++---- include/linux/efi.h | 3 +- 2 files changed, 102 insertions(+), 12 deletions(-)
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 25d6d35..8717ea9 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -146,6 +146,13 @@ 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 *); +static DECLARE_WORK(efivar_work, efivar_update_sysfs_entry); + /* Return the number of unicode characters in data */ static unsigned long utf16_strnlen(efi_char16_t *s, size_t maxlength) @@ -732,9 +739,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]; @@ -743,14 +747,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); + schedule_work(&efivar_work); *id = part; return ret; @@ -1204,6 +1201,97 @@ 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); + } +} + /* * For now we register the efi subsystem with the firmware subsystem * and the vars subsystem with the efi subsystem. In the future, it @@ -1259,6 +1347,7 @@ static void __exit efivars_exit(void) { if (efi_enabled) { + cancel_work_sync(&efivar_work); unregister_efivars(&__efivars); kobject_put(efi_kobj); } diff --git a/include/linux/efi.h b/include/linux/efi.h index 103adc6..cecdf58 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
| |