From: Rafael J. Wysocki on
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(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
On Mon, 3 May 2010, Rafael J. Wysocki wrote:

> 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.

How about changing the contents of /sys/power/state depending on the
current policy? When the policy is "forced" it should look the same as
it does now. When the policy is "opportunistic" it should contain
"mem" and "on".

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Arve Hjønnevåg on
On Mon, May 3, 2010 at 3:11 PM, Alan Stern <stern(a)rowland.harvard.edu> wrote:
> On Mon, 3 May 2010, Rafael J. Wysocki wrote:
>
>> 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.
>
> How about changing the contents of /sys/power/state depending on the
> current policy? �When the policy is "forced" it should look the same as
> it does now. �When the policy is "opportunistic" it should contain
> "mem" and "on".

It already does this.

--
Arve Hj�nnev�g
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
On Tue, 27 Apr 2010, [UTF-8] Arve Hjønnevåg wrote:

> +static void suspend_worker(struct work_struct *work)
> +{
> + int ret;
> + int entry_event_num;
> +
> + enable_suspend_blockers = true;
> + while (!suspend_is_blocked()) {
> + entry_event_num = current_event_num;
> +
> + if (debug_mask & DEBUG_SUSPEND)
> + pr_info("suspend: enter suspend\n");
> +
> + ret = pm_suspend(requested_suspend_state);
> +
> + if (debug_mask & DEBUG_EXIT_SUSPEND)
> + pr_info_time("suspend: exit suspend, ret = %d ", ret);
> +
> + if (current_event_num == entry_event_num)
> + pr_info("suspend: pm_suspend returned with no event\n");
> + }
> + enable_suspend_blockers = false;
> +}

Here's a completely new issue. When using opportunistic suspends on an
SMP system, it could happen that the system gets a wakeup event and
this routine starts running again before the event's IRQ handler has
finished (or has enabled a suspend blocker). The system would
re-suspend too soon.

Is there anything in the PM core that waits until all outstanding IRQs
have been handled before unfreezing processes? If there is, I can't
find it. Should such a thing be added?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Rafael J. Wysocki on
On Thursday 06 May 2010, Alan Stern wrote:
> On Tue, 27 Apr 2010, [UTF-8] Arve Hjønnevåg wrote:
>
> > +static void suspend_worker(struct work_struct *work)
> > +{
> > + int ret;
> > + int entry_event_num;
> > +
> > + enable_suspend_blockers = true;
> > + while (!suspend_is_blocked()) {
> > + entry_event_num = current_event_num;
> > +
> > + if (debug_mask & DEBUG_SUSPEND)
> > + pr_info("suspend: enter suspend\n");
> > +
> > + ret = pm_suspend(requested_suspend_state);
> > +
> > + if (debug_mask & DEBUG_EXIT_SUSPEND)
> > + pr_info_time("suspend: exit suspend, ret = %d ", ret);
> > +
> > + if (current_event_num == entry_event_num)
> > + pr_info("suspend: pm_suspend returned with no event\n");
> > + }
> > + enable_suspend_blockers = false;
> > +}
>
> Here's a completely new issue. When using opportunistic suspends on an
> SMP system, it could happen that the system gets a wakeup event and
> this routine starts running again before the event's IRQ handler has
> finished (or has enabled a suspend blocker). The system would
> re-suspend too soon.

This routine will be run from a freezable workqueue.

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