Re: [ 06/23] [Suspend2] Disable usermode helper invocations when the freezer is on.

["Rafael J. Wysocki" <rjw@sisk.pl>]

Hi,

On Wednesday 01 February 2006 00:21, Nigel Cunningham wrote:
> On Tuesday 31 January 2006 20:58, Rafael J. Wysocki wrote:
> > Hi,
> > > > >
> > > > > +	if (freezer_is_on())
> > > > > +		return 0;
> > > > > +
> > > > >  	if (path[0] == '\0')
> > > > >  		return 0;
> > > >
> > > > Disabling the usermode helper while freeze_processes() is executed
> > > > seems to be a good idea to me, but I think it should be done with a
> > > > mutex or something like that.
> > >
> > > With the refrigerator code you guys are using at the moment, ouldn't that
> > > result in deadlocks when we later try to freeze the process in
> > > preparation for the atomic restore? (Or perhaps you don't freeze
> > > processes at that point?)
> >
> > I'm not sure what you mean.  I said "mutex" because you seem to have a race
> > here (the freezer may be started right after the freezer_is_on() check). 
> > IMO the freezer should disable the invocations of new usermode helpers and
> > wait util all of the already running helpers are finished.  For this
> > purpose two variables would be needed and a lock.
> 
> Sorry. Being a bit thick.
> 
> I wasn't worried about already-running usermode helpers (or about-to-run 
> helpers either) because the spawned processes would either complete or be 
> caught be the usual freezing code. My concern was more that new invocations 
> of this path don't leave us with unfrozen processes hanging around. That 
> said, I think you have a valid point about letting existing helpers run to 
> completion. It does make me concerned though about the possibility of a 
> long-lived helper (not that I know that there really are such things at the 
> moment). Do you think that needs to be taken as a genuine concern? If so, I 
> guess we then need to make freezing a four stage process:
> 
> 1. Stop new usermodehelpers from starting & let existing ones run to 
> completion.
> 2. Freeze userspace.
> 3. Freezer bdevs.
> 4. Freezer kernel space.

Well, I know a little about bdevs, but generally I think that's reasonable.

Greetings,
Rafael