[flashrom] [Patch] Add coarse-grained IPC locking mechanism to Flashrom

[David Hendricks]

On Wed, Feb 2, 2011 at 12:58 AM, Mathias Krause
<mathias.krause at secunet.com>wrote:

> On 01.02.2011 22:37, David Hendricks wrote:
> > Hi,
> > I have attached a patch which adds a locking mechanism to Flashrom to
> > prevent multiple instances from running simultaneously. It may need some
> > modification to fit different Flashrom use cases better, but I think
> > it's a good start.
>
> Nice


Thanks!

On Wed, Feb 2, 2011 at 12:58 AM, Mathias Krause <mathias.krause at secunet.com>
 wrote:

> .> The size of the patch is somewhat misleading -- The only real changes to
> > Flashrom's current code are some of the exit points in cli_classic() and
> > some added stuff in the Makefile. Everything else is contained in new
> > files. The new files are:
> > csem.{c,h}: Low-level code that interfaces with semctl(), semget(), etc.
> > ipc_lock.{c,h}: Wrapper for csem stuff.
> > locks.h: A listing of locks that Flashrom cares about.
> > big_lock.{c,h}: An even higher-level wrapper around ipc_lock stuff,
> > useful for simple, coarse-grained locking.
>
> Well, quite some wrapping wrappers :/
>

Agreed, to a point. The ipc_lock stuff is a useful abstraction. The big_lock
stuff is just there to emphasize the notion of a single lock used for
multiple programs -- It's just a trivial wrapper for handling a single lock.

>From Flashrom's perspective this probably isn't very useful. Perhaps that's
reason enough to omit it. It's mostly just there to help coordinate between
different programs (ie for distro maintainers).

On Wed, Feb 2, 2011 at 12:58 AM, Mathias Krause <mathias.krause at secunet.com>
 wrote:

> > util/use_big_lock.sh: Used by the Makefile to test for POSIX.1-2001
> > compliance.
>
> That test should be integrated into the Makefile, no?
>

Maybe. I spent some time trying to do that but my result came out fugly.

IIRC the hard part was trying to define a macro based on the result of the
command. The other tests do the opposite -- They use an existing macro to
determine whether to run a test. Someone with stronger Make-fu is welcome to
show me how this is done :-)

On Wed, Feb 2, 2011 at 12:58 AM, Mathias Krause <mathias.krause at secunet.com>
 wrote:

> Otherwise this code looks good to me, albeit a little bit bloated for
> just such little functionality. Anyway, Carl-Daniel should comment on this.
>

Fair enough. I feel it's useful and wanted to get this upstreamed to reduce
diffs w/ Chromium OS. But it is pretty heavy-weight for what might not be
important to the upstream flashrom community at this time.

To illustrate our usage case, on Chrome OS devices* we have multiple
programs that access the ROM. Our auto-updater uses these programs in the
background. We've already been bitten by developers attempting to manually
update/reflash their BIOS and/or EC firmware while the auto-updater is
attempting to do stuff. ECs can be especially troublesome since they often
access their ROM constantly and will crash if you corrupt the ROM, unlike
most x86 BIOSes where you at least stand a chance to recover.

*Chrome OS is tailored for specific devices. Chromium OS is the more generic
version that can be put on a broad range of hardware.

-- 
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20110202/f5d97ab3/attachment.html>