Messages in this thread | | | Date | Fri, 23 Apr 2010 12:45:41 -0400 (EDT) | From | Alan Stern <> | Subject | Re: [linux-pm] [PATCH 1/9] PM: Add suspend block api. |
| |
On Fri, 23 Apr 2010, Alan Stern wrote:
> On Thu, 22 Apr 2010, [UTF-8] Arve Hjønnevåg wrote:
> > +struct suspend_blocker { > > +#ifdef CONFIG_SUSPEND_BLOCKERS > > + atomic_t flags; > > + const char *name; > > +#endif > > Why is flags an atomic_t? Are you worried that drivers might try to > activate a suspend_blocker at the same time that it is being destroyed? > If this happens, does the code do the right thing? I don't think it > does -- if a race occurs, suspend_block() will leave flags set to the > wrong value. The same goes for suspend_unblock(). > > Since these routines don't nest, there is also the possibility of a > race between suspend_block() and suspend_unblock(). If the race goes > one way the blocker is active; the other way it isn't. Given that such > problems already exist, why worry about what happens when the suspend > blocker is destroyed?
Having now read the later patches, I see that you switch over to using a spinlock instead of an atomic_t. My suggestion is to use a spinlock right from the start. It will be less confusing.
Alan Stern
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |