On Wed, Feb 2, 2011 at 12:58 AM, Mathias Krause
<mathias.krause@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!
.> 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).
> 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 :-)
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.