Re: [PATCH 1/8] PM: Add suspend block api.

[Rafael J. Wysocki]

On Monday 03 May 2010, Pavel Machek wrote:
> Hi!
> 
> > > > > As I explained before (and got no reply),  the proposed interface is
> > > > > ugly. It uses one sysfs file to change semantics of another one.
> > > > 
> > > > In fact this behavior was discussed at the LF Collab Summit and no one
> > > > involved had any problem with that.
> > > 
> > > Well, I explained why I disliked in previous mail in more details,
> > 
> > We do exactly the same thing with 'pm_test', so I'm not sure what the problem is.
> > 
> > > and neither you nor Arve explained why it is good solution.
> > 
> > Because it's less confusing.  Having two different attributes returning
> > almost the same contents and working in a slightly different way wouldn't be
> > too clean IMO.
> 
> No, I don't think it is similar to pm_test. pm_test is debug-only, and
> orthogonal to state -- all combinations make sense.
> 
> With 'oportunistic > policy', state changes from blocking to
> nonblocking (surprise!). Plus, it is not orthogonal:
> 
> (assume we added s-t-flash on android for powersaving... or imagine I
> get oportunistic suspend working on PC --I was there with limited
> config on x60).
> 
> policy: oportunistic forced
> 
> state: on mem disk
> 
> First disadvantage of proposed interface is that while 'opportunistic
> mem' is active, I can't do 'forced disk' to save bit more power.
> 
> Next, not all combinations make sense.
> 
> oportunistic on == forced <nothing>

That's correct, but the "opportunistic on" thing is there to avoid switching
back and forth from "opportunistic" to "forced".  So that's a matter of
convenience rather than anything else.

> oportunistic disk -- probably something that will not be implemented
> any time soon.

Yes, and that's why "disk" won't work with "opportunistic" for now.

> oportunistic mem -- makes sense.
> 
> forced on -- NOP

There won't be "on" with "forced" (that probably needs changing at the moment).

> forced mem  -- makes sense.
> 
> forced disk -- makes sense.
> 
> So we have matrix of 7 possibilities, but only 4 make sense... IMO its confusing.

The main problem is that the entire suspend subsystem is going to work in a
different way when suspend blockers are enforced.  Thus IMO it makes sense to
provide a switch between the "opportunistic" and "forced" modes, because that
clearly indicates to the user (or user space in general) how the whole suspend
subsystem actually works at the moment.

As long as it's "opportunistic", the system will autosuspend if suspend
blockers are not active and the behavior of "state" reflects that.  If you want
to enforce a transition, switch to "forced" first.

That's not at all confusing if you know what you're doing.  The defailt mode is
"forced", so the suspend subsystem works "as usual" by default.  You have to
directly switch it to "opportunistic" to change the behavior and once you've
done that, you shouldn't really be surprised that the behavior has changed.
That's what you've requested after all.

So no, I don't really think it's confusing.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/