Messages in this thread | | | From | Mike Waychison <> | Date | Tue, 14 Aug 2012 11:04:00 -0700 | Subject | Re: efi_pstore: question about how to remove create_sysfs_entry() from a write callback. |
| |
On Tue, Aug 14, 2012 at 9:05 AM, Seiji Aguchi <seiji.aguchi@hds.com> wrote: > Hi, > > I'm sending an email to discuss how to remove create_sysfs_entry() from a write callback. > > [Problem] > > Current 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. > > To resolve the problem above, my goal here is removing create_sysfs_entry() from a write callback. > > [Ideas] > > (1) Introduce a workqueue updating sysfs entries > > To remove create_sysfs_entry() from a write callback, > It seems to be possible if efi_pstore updates its sysfs files > by scanning existing entries in NVRAM with a GetNextVariable() > in a workqueue. > > > I created a prototype patch based on an idea above but can't avoid a race > between SetVariable() in a write callback and GetNextVariable() in a workqueue. > It is not guaranteed by EFI specification. > > EFI 2.3.1 specification, page 217. > <snip> > Calls to SetVariable() between calls to > GetNextVariableName() may produce unpredictable results. > <snip>
Can we not serialize this with &efivars->lock if it is updated to disable interrupts? A loop in the workqueue that locks, iterates through ->get_next_variable, and compares against searches in efivars->list should work, no?
> > > (2) Don't support sysfs entries in efi_pstore. > > Another idea is _not_ updating sysfs entries at all in efi_pstore. > This can avoid a race SetVariable() and GetNextVariable(). > > write callback > - simply write a new entry with SetVariable(). > - This fits a discussion about holding multiple logs in a thread below. > http://marc.info/?l=linux-kernel&m=134316268011854&w=2 > > erase callback > - simply erase an existing entry with SetVariable(). > > read callback > - Scaning existing entries with GetNextVariable(). > We can avoid a race between GetNextVariable() in a read callback > and SetVariable() in a write/erase callback by protecting them with efi_lock. > > IMO, idea (2) is reasonable because we already have an interface, /dev/pstore, which users can access > to NVRAM and we don't need to support multiple user interfaces. > > Any comments are welcome. > > Seiji
| |