lkml.org 
[lkml]   [2012]   [Sep]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RESEND][PATCH v4 2/3] efi_pstore: Introducing workqueue updating sysfs entries
    Date
    [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, are not 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 <seiji.aguchi@hds.com>
    ---
    drivers/firmware/efivars.c | 119 +++++++++++++++++++++++++++++++++++++++----
    include/linux/efi.h | 3 +-
    2 files changed, 110 insertions(+), 12 deletions(-)

    diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
    index bd1df01..cd16ea6 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)
    @@ -731,9 +738,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];

    @@ -742,14 +746,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;
    @@ -1200,6 +1197,104 @@ EXPORT_SYMBOL_GPL(register_efivars);
    static struct efivars __efivars;
    static struct efivar_operations ops;

    +static void delete_all_stale_sysfs_entries(void)
    +{
    + struct efivars *efivars = &__efivars;
    + struct efivar_entry *entry, *n, *found;
    + efi_status_t status;
    + unsigned long flags;
    +
    + while (1) {
    + found = NULL;
    + spin_lock_irqsave(&efivars->lock, flags);
    + list_for_each_entry_safe(entry, n, &efivars->list, list) {
    + status = get_var_data_locked(efivars, &entry->var);
    + if (status != EFI_SUCCESS) {
    + found = entry;
    + list_del(&entry->list);
    + break;
    + }
    + }
    + spin_unlock_irqrestore(&efivars->lock, flags);
    + if (found)
    + efivar_unregister(entry);
    + else
    + break;
    + }
    +}
    +
    +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 stale sysfs entries */
    + delete_all_stale_sysfs_entries();
    +
    + /* 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
    @@ -1254,6 +1349,8 @@ err_put:
    static void __exit
    efivars_exit(void)
    {
    + cancel_work_sync(&efivar_work);
    +
    if (efi_enabled) {
    unregister_efivars(&__efivars);
    kobject_put(efi_kobj);
    diff --git a/include/linux/efi.h b/include/linux/efi.h
    index ec45ccd..8e03cc0 100644
    --- a/include/linux/efi.h
    +++ b/include/linux/efi.h
    @@ -643,7 +643,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

    \
     
     \ /
      Last update: 2012-09-05 01:02    [W:2.642 / U:1.176 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site