Matt, > There isn't really a formalised interface for dealing with EFI > variables or struct efivar_entry. This has led to the efivarfs code > directly accessing '__efivars'. I agree with you. I have some comment on a pstore part. > @@ -1363,86 +1195,90 @@ static int efi_pstore_write(enum pstore_type_id type, > for (i = 0; i < DUMP_NAME_LEN; i++) > efi_name[i] = name[i]; > > - efivars->ops->set_variable(efi_name, &vendor, PSTORE_EFI_ATTRIBUTES, > - size, psi->buf); > + entry = kzalloc(sizeof(*entry), GFP_KERNEL); > + if (!entry) > + return -ENOMEM; Please don't allocate memory dynamically in the efi_pstore_write() because efi_pstore_write() is called in an interrupt context like oops and panic. Also, a whole process of query_info() + entry_set() should be protected by a single spin lock() Because if efi_pstore_write() runs concurrently, a result of query_info() may be meaningless... So, how about introducing query_info_nolock and entry_set_nolock()? spin_lock() query_info_nolock() entry_set_nolock() spin_unlock() IMO, efivarfs_file_write() should be fixed as above. As for efi_pstore_erase() and efi_pstore_read(), it will take some time to fully understand your change. Seiji -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
