syncing the disks when entering sleep

syncing the disks when entering sleep

[Leisner, Martin]

I noticed when we enter sleep states, it seems there's always a
sys_sync given.

When having an embedded system doing WOL (already in laptop mode to
cache
all the writes), it reasonable to wake up dozens of times of day and
quickly go back to sleep.  If the disk is not spun up,
and there's some work to be done, won't the sys_sync spin up the disk
before going to sleep?

It would be good to have a knob (in /sys) you can tweak from usespace to

configure the behavior of whether you want to do a sync when entering
sleep.

The ideal behavior would be:

    if(disk is spun up)
        then let the sync happen

marty

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Friday 22 January 2010, Leisner, Martin wrote:
> I noticed when we enter sleep states, it seems there's always a
> sys_sync given.
> 
> When having an embedded system doing WOL (already in laptop mode to
> cache
> all the writes), it reasonable to wake up dozens of times of day and
> quickly go back to sleep.  If the disk is not spun up,
> and there's some work to be done, won't the sys_sync spin up the disk
> before going to sleep?
> 
> It would be good to have a knob (in /sys) you can tweak from usespace to
> 
> configure the behavior of whether you want to do a sync when entering
> sleep.
> 
> The ideal behavior would be:
> 
>     if(disk is spun up)
>         then let the sync happen

I'm not against that.  Patch welcome. :-)

Rafael

Re: syncing the disks when entering sleep

[Nigel Cunningham]

Hi.

Rafael J. Wysocki wrote:
> On Friday 22 January 2010, Leisner, Martin wrote:
>> I noticed when we enter sleep states, it seems there's always a
>> sys_sync given.
>>
>> When having an embedded system doing WOL (already in laptop mode to
>> cache
>> all the writes), it reasonable to wake up dozens of times of day and
>> quickly go back to sleep.  If the disk is not spun up,
>> and there's some work to be done, won't the sys_sync spin up the disk
>> before going to sleep?
>>
>> It would be good to have a knob (in /sys) you can tweak from usespace to
>>
>> configure the behavior of whether you want to do a sync when entering
>> sleep.
>>
>> The ideal behavior would be:
>>
>>     if(disk is spun up)
>>         then let the sync happen
> 
> I'm not against that.  Patch welcome. :-)

Oooh. I am. That's providing potential for unpredictable behaviour. Much
better IMO would be providing a tuning know that explicitly and
unconditionally disables syncing.

Regards,

Nigel

Re: syncing the disks when entering sleep

[Leisner, Martin]

To clarify what I proposed:

if(user sets knobs) 
    if(disk is spun up)
         sync

currently the algorithm is

    sync

marty

>   -----Original Message-----
>   From: Nigel Cunningham [mailto:ncunningham@crca.org.au]
>   Sent: Friday, January 22, 2010 4:32 PM
>   To: Rafael J. Wysocki
>   Cc: Leisner, Martin; linux-pm@lists.linux-foundation.org
>   Subject: Re: [linux-pm] syncing the disks when entering sleep
>   
>   Hi.
>   
>   Rafael J. Wysocki wrote:
>   > On Friday 22 January 2010, Leisner, Martin wrote:
>   >> I noticed when we enter sleep states, it seems there's always a
>   >> sys_sync given.
>   >>
>   >> When having an embedded system doing WOL (already in laptop mode to
>   >> cache
>   >> all the writes), it reasonable to wake up dozens of times of day and
>   >> quickly go back to sleep.  If the disk is not spun up,
>   >> and there's some work to be done, won't the sys_sync spin up the disk
>   >> before going to sleep?
>   >>
>   >> It would be good to have a knob (in /sys) you can tweak from usespace
>   to
>   >>
>   >> configure the behavior of whether you want to do a sync when entering
>   >> sleep.
>   >>
>   >> The ideal behavior would be:
>   >>
>   >>     if(disk is spun up)
>   >>         then let the sync happen
>   >
>   > I'm not against that.  Patch welcome. :-)
>   
>   Oooh. I am. That's providing potential for unpredictable behaviour. Much
>   better IMO would be providing a tuning know that explicitly and
>   unconditionally disables syncing.
>   
>   Regards,
>   
>   Nigel

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Friday 22 January 2010, Nigel Cunningham wrote:
> Hi.
> 
> Rafael J. Wysocki wrote:
> > On Friday 22 January 2010, Leisner, Martin wrote:
> >> I noticed when we enter sleep states, it seems there's always a
> >> sys_sync given.
> >>
> >> When having an embedded system doing WOL (already in laptop mode to
> >> cache
> >> all the writes), it reasonable to wake up dozens of times of day and
> >> quickly go back to sleep.  If the disk is not spun up,
> >> and there's some work to be done, won't the sys_sync spin up the disk
> >> before going to sleep?
> >>
> >> It would be good to have a knob (in /sys) you can tweak from usespace to
> >>
> >> configure the behavior of whether you want to do a sync when entering
> >> sleep.
> >>
> >> The ideal behavior would be:
> >>
> >>     if(disk is spun up)
> >>         then let the sync happen
> > 
> > I'm not against that.  Patch welcome. :-)
> 
> Oooh. I am. That's providing potential for unpredictable behaviour. Much
> better IMO would be providing a tuning know that explicitly and
> unconditionally disables syncing.

You're right, sorry.

Yes, a knob that disables syncing unconditionally was the thing I was thinking
about.

Rafael

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Friday 22 January 2010, Leisner, Martin wrote:
> To clarify what I proposed:
> 
> if(user sets knobs) 
>     if(disk is spun up)
>          sync
> 
> currently the algorithm is
> 
>     sync

Yes, but Nigel said it should be

if(user knob is unset)
          sync

which I agree with.

[BTW, please don't top-post in future.]

Rafael

Re: syncing the disks when entering sleep

[Leisner, Martin]

>   -----Original Message-----
>   From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
>   Sent: Friday, January 22, 2010 4:38 PM
>   To: Leisner, Martin
>   Cc: Nigel Cunningham; linux-pm@lists.linux-foundation.org
>   Subject: Re: [linux-pm] syncing the disks when entering sleep
>   
>   On Friday 22 January 2010, Leisner, Martin wrote:
>   > To clarify what I proposed:
>   >
>   > if(user sets knobs)
>   >     if(disk is spun up)
>   >          sync
>   >
>   > currently the algorithm is
>   >
>   >     sync
>   
>   Yes, but Nigel said it should be
>   
>   if(user knob is unset)
>             sync
>   
>   which I agree with.
>   
>   [BTW, please don't top-post in future.]
>   
>   Rafael

if(knob is set) {
    if(disk is spun up)
       sync
} else sync

The spinup of the disk can have a very noticeable amount of time...if the user doesn't care about syncing, then the top is preferred...


marty

Re: syncing the disks when entering sleep

[Nigel Cunningham]

Hi.

Leisner, Martin wrote:
> 
>>   -----Original Message-----
>>   From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
>>   Sent: Friday, January 22, 2010 4:38 PM
>>   To: Leisner, Martin
>>   Cc: Nigel Cunningham; linux-pm@lists.linux-foundation.org
>>   Subject: Re: [linux-pm] syncing the disks when entering sleep
>>   
>>   On Friday 22 January 2010, Leisner, Martin wrote:
>>   > To clarify what I proposed:
>>   >
>>   > if(user sets knobs)
>>   >     if(disk is spun up)
>>   >          sync
>>   >
>>   > currently the algorithm is
>>   >
>>   >     sync
>>   
>>   Yes, but Nigel said it should be
>>   
>>   if(user knob is unset)
>>             sync
>>   
>>   which I agree with.
>>   
>>   [BTW, please don't top-post in future.]
>>   
>>   Rafael
> 
> if(knob is set) {
>     if(disk is spun up)
>        sync
> } else sync
> 
> The spinup of the disk can have a very noticeable amount of time...if the user doesn't care about syncing, then the top is preferred...

That sounds much better; thanks.

As an aside, I think the writeback code needs some tweaking in general.
I don't know about you guys, but I'm seeing that sync take far longer
than I reckon it should sometimes. Just a gut feeling - not based on
having looked at /proc/meminfo first - but I'd be interested to know if
you think the same.

Nigel

Re: syncing the disks when entering sleep

[Nigel Cunningham]

Hi again.

Nigel Cunningham wrote:
> Hi.
> 
> Leisner, Martin wrote:
>>>   -----Original Message-----
>>>   From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
>>>   Sent: Friday, January 22, 2010 4:38 PM
>>>   To: Leisner, Martin
>>>   Cc: Nigel Cunningham; linux-pm@lists.linux-foundation.org
>>>   Subject: Re: [linux-pm] syncing the disks when entering sleep
>>>   
>>>   On Friday 22 January 2010, Leisner, Martin wrote:
>>>   > To clarify what I proposed:
>>>   >
>>>   > if(user sets knobs)
>>>   >     if(disk is spun up)
>>>   >          sync
>>>   >
>>>   > currently the algorithm is
>>>   >
>>>   >     sync
>>>   
>>>   Yes, but Nigel said it should be
>>>   
>>>   if(user knob is unset)
>>>             sync
>>>   
>>>   which I agree with.
>>>   
>>>   [BTW, please don't top-post in future.]
>>>   
>>>   Rafael
>> if(knob is set) {
>>     if(disk is spun up)
>>        sync
>> } else sync
>>
>> The spinup of the disk can have a very noticeable amount of time...if the user doesn't care about syncing, then the top is preferred...

Sorry to reply to myself but...

> That sounds much better; thanks.

I shouldn't have said that - as Rafael said

if (knob is unset)
  sync

provides much more predictable behaviour. I know you want the code to
look at whether the disk is spun up, but that makes things more
unpredictable. Imagine a situation where you get some error in which
whether we synced or not is important - reproducing, diagnosing and
fixing the issue gets more complicated if we go your way.

The other issue would be detecting whether disks (there might be more
than one!) are spun up. That, in itself, would probably require a lot of
extra code to get the info from the drivers.

An explicit and unconditional disabling of syncing is simpler.

Regards,

Nigel

Re: syncing the disks when entering sleep

[Pavel Machek]

Hi!

> > I noticed when we enter sleep states, it seems there's always a
> > sys_sync given.
> > 
> > When having an embedded system doing WOL (already in laptop mode to
> > cache
> > all the writes), it reasonable to wake up dozens of times of day and
> > quickly go back to sleep.  If the disk is not spun up,
> > and there's some work to be done, won't the sys_sync spin up the disk
> > before going to sleep?
> > 
> > It would be good to have a knob (in /sys) you can tweak from usespace to
> > 
> > configure the behavior of whether you want to do a sync when entering
> > sleep.
> > 
> > The ideal behavior would be:
> > 
> >     if(disk is spun up)
> >         then let the sync happen
> 
> I'm not against that.  Patch welcome. :-)

I'd say such knob would be ugly. But maybe acceptable way would be
echo mem-nosync > power, or maybe it can already be done using s2disk
ioctl interface...?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Pavel Machek]

> > > > The ideal behavior would be:
> > > > 
> > > >     if(disk is spun up)
> > > >         then let the sync happen
> > > 
> > > I'm not against that.  Patch welcome. :-)
> > 
> > I'd say such knob would be ugly.
> 
> Define "ugly", please.

Per-system property, which should better be
per-program-that-requires-suspend. You request suspend without syncing
(you want it quick, battery is 90%), then the battery runs low, and
system daeomn requests s2ram, not realizing that someone disabled sync
from under him.

> > But maybe acceptable way would be echo mem-nosync > power, or maybe it can
> > already be done using s2disk ioctl interface...?
> 
> Nope.

Nope what?

AFAICT no new interface is needed. Just do SNAPSHOT_FREEZE, then
_S2RAM then _UNFREEZE.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Tuesday 26 January 2010, Pavel Machek wrote:
> Hi!
> 
> > > I noticed when we enter sleep states, it seems there's always a
> > > sys_sync given.
> > > 
> > > When having an embedded system doing WOL (already in laptop mode to
> > > cache
> > > all the writes), it reasonable to wake up dozens of times of day and
> > > quickly go back to sleep.  If the disk is not spun up,
> > > and there's some work to be done, won't the sys_sync spin up the disk
> > > before going to sleep?
> > > 
> > > It would be good to have a knob (in /sys) you can tweak from usespace to
> > > 
> > > configure the behavior of whether you want to do a sync when entering
> > > sleep.
> > > 
> > > The ideal behavior would be:
> > > 
> > >     if(disk is spun up)
> > >         then let the sync happen
> > 
> > I'm not against that.  Patch welcome. :-)
> 
> I'd say such knob would be ugly.

Define "ugly", please.

> But maybe acceptable way would be echo mem-nosync > power, or maybe it can
> already be done using s2disk ioctl interface...?

Nope.

Rafael

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Tuesday 26 January 2010, Pavel Machek wrote:
> 
> > > > > The ideal behavior would be:
> > > > > 
> > > > >     if(disk is spun up)
> > > > >         then let the sync happen
> > > > 
> > > > I'm not against that.  Patch welcome. :-)
> > > 
> > > I'd say such knob would be ugly.
> > 
> > Define "ugly", please.
> 
> Per-system property, which should better be
> per-program-that-requires-suspend. You request suspend without syncing
> (you want it quick, battery is 90%), then the battery runs low, and
> system daeomn requests s2ram, not realizing that someone disabled sync
> from under him.

I really prefer a per-system setting.  The program that wants to sync anyway
can easily do that by itself.

> > > But maybe acceptable way would be echo mem-nosync > power, or maybe it can
> > > already be done using s2disk ioctl interface...?
> > 
> > Nope.
> 
> Nope what?
> 
> AFAICT no new interface is needed. Just do SNAPSHOT_FREEZE, then
> _S2RAM then _UNFREEZE.

That's not quite straightforward and I wouldn't seriously suggest that to
anyone.

Rafael

Re: syncing the disks when entering sleep

[Pavel Machek]

On Wed 2010-01-27 01:51:03, Rafael J. Wysocki wrote:
> On Tuesday 26 January 2010, Pavel Machek wrote:
> > 
> > > > > > The ideal behavior would be:
> > > > > > 
> > > > > >     if(disk is spun up)
> > > > > >         then let the sync happen
> > > > > 
> > > > > I'm not against that.  Patch welcome. :-)
> > > > 
> > > > I'd say such knob would be ugly.
> > > 
> > > Define "ugly", please.
> > 
> > Per-system property, which should better be
> > per-program-that-requires-suspend. You request suspend without syncing
> > (you want it quick, battery is 90%), then the battery runs low, and
> > system daeomn requests s2ram, not realizing that someone disabled sync
> > from under him.
> 
> I really prefer a per-system setting.  The program that wants to sync anyway
> can easily do that by itself.

Well, existing programs expect existing behaviour... Programs that do
not want to sync can easily do it themselves, too, without afecting
rest of system. 

> > > Nope.
> > 
> > Nope what?
> > 
> > AFAICT no new interface is needed. Just do SNAPSHOT_FREEZE, then
> > _S2RAM then _UNFREEZE.
> 
> That's not quite straightforward and I wouldn't seriously suggest that to
> anyone.

Three ioctls, seems pretty much ok.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Nigel Cunningham]

Hi.

Pavel Machek wrote:
> On Wed 2010-01-27 01:51:03, Rafael J. Wysocki wrote:
>> On Tuesday 26 January 2010, Pavel Machek wrote:
>>>>>>> The ideal behavior would be:
>>>>>>>
>>>>>>>     if(disk is spun up)
>>>>>>>         then let the sync happen
>>>>>> I'm not against that.  Patch welcome. :-)
>>>>> I'd say such knob would be ugly.
>>>> Define "ugly", please.
>>> Per-system property, which should better be
>>> per-program-that-requires-suspend. You request suspend without syncing
>>> (you want it quick, battery is 90%), then the battery runs low, and
>>> system daeomn requests s2ram, not realizing that someone disabled sync
>>> from under him.
>> I really prefer a per-system setting.  The program that wants to sync anyway
>> can easily do that by itself.
> 
> Well, existing programs expect existing behaviour... Programs that do
> not want to sync can easily do it themselves, too, without afecting
> rest of system. 

What's all this talk about 'programs'? The direct invokers of the
hibernation and suspend-to-ram code are few in number. They're easily
modified to match changes in the kernel code, and when it comes to
syncing, there's no need for the modification to be made in lock-step.
If you hibernate or suspend without having dirty pages synced to disk
first, it shouldn't matter, should it? Resuming is pretty reliable, and
even if it fails, journal recovery is not problematic.

Regards,

Nigel

Re: syncing the disks when entering sleep

[Pavel Machek]

> > Per-system property, which should better be
> > per-program-that-requires-suspend. You request suspend without syncing
> > (you want it quick, battery is 90%), then the battery runs low, and
> > system daeomn requests s2ram, not realizing that someone disabled sync
> > from under him.
> 
> I really prefer a per-system setting.  The program that wants to sync anyway
> can easily do that by itself.

Yes, but existing apps do not know they have to sync. You are
essentially adding "break back compatibility" system wide option, when
better alternative exists... See above for concrete example where it
may hurt.

> > Nope what?
> > 
> > AFAICT no new interface is needed. Just do SNAPSHOT_FREEZE, then
> > _S2RAM then _UNFREEZE.
> 
> That's not quite straightforward and I wouldn't seriously suggest that to
> anyone.

Well, seems more straightforward than running around and adding sync
to all the scripts/programs just to keep existing behaviour.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Wednesday 27 January 2010, Pavel Machek wrote:
> 
> > > Per-system property, which should better be
> > > per-program-that-requires-suspend. You request suspend without syncing
> > > (you want it quick, battery is 90%), then the battery runs low, and
> > > system daeomn requests s2ram, not realizing that someone disabled sync
> > > from under him.
> > 
> > I really prefer a per-system setting.  The program that wants to sync anyway
> > can easily do that by itself.
> 
> Yes, but existing apps do not know they have to sync. You are
> essentially adding "break back compatibility" system wide option, when
> better alternative exists... See above for concrete example where it
> may hurt.

I don't get what the problem is, really.

There's _nothing_ here that breaks the existing behavior.  If the user doesn't
set the switch, everything works as usual.  If he does, breaking the "back
compatibility" is _his_ problem.

Rafael

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Wednesday 27 January 2010, Pavel Machek wrote:
> On Wed 2010-01-27 01:51:03, Rafael J. Wysocki wrote:
> > On Tuesday 26 January 2010, Pavel Machek wrote:
...
> > > 
> > > AFAICT no new interface is needed. Just do SNAPSHOT_FREEZE, then
> > > _S2RAM then _UNFREEZE.
> > 
> > That's not quite straightforward and I wouldn't seriously suggest that to
> > anyone.
> 
> Three ioctls, seems pretty much ok.

They are for something else.  Yes, you could use them for this purpose,
incidentally.  No, you shouldn't do that.  OK?

Please note that merely _opening_ /dev/snapshot has side effects, so that's
not just "three ioctls".

Rafael

Re: syncing the disks when entering sleep

[Pavel Machek]

> On Wednesday 27 January 2010, Pavel Machek wrote:
> > On Wed 2010-01-27 01:51:03, Rafael J. Wysocki wrote:
> > > On Tuesday 26 January 2010, Pavel Machek wrote:
> ...
> > > > 
> > > > AFAICT no new interface is needed. Just do SNAPSHOT_FREEZE, then
> > > > _S2RAM then _UNFREEZE.
> > > 
> > > That's not quite straightforward and I wouldn't seriously suggest that to
> > > anyone.
> > 
> > Three ioctls, seems pretty much ok.
> 
> They are for something else.  Yes, you could use them for this purpose,
> incidentally.  No, you shouldn't do that.  OK?

Why not? It seems less ugly then the other solution.

> Please note that merely _opening_ /dev/snapshot has side effects, so that's
> not just "three ioctls".

I search Doc*/power/userland-swsusp.txt, and could not find mentions
of sideeffects. Perhaps that should be fixed?
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Thursday 28 January 2010, Pavel Machek wrote:
> > On Wednesday 27 January 2010, Pavel Machek wrote:
> > > On Wed 2010-01-27 01:51:03, Rafael J. Wysocki wrote:
> > > > On Tuesday 26 January 2010, Pavel Machek wrote:
> > ...
> > > > > 
> > > > > AFAICT no new interface is needed. Just do SNAPSHOT_FREEZE, then
> > > > > _S2RAM then _UNFREEZE.
> > > > 
> > > > That's not quite straightforward and I wouldn't seriously suggest that to
> > > > anyone.
> > > 
> > > Three ioctls, seems pretty much ok.
> > 
> > They are for something else.  Yes, you could use them for this purpose,
> > incidentally.  No, you shouldn't do that.  OK?
> 
> Why not? It seems less ugly then the other solution.

Well, not to me at least.

> > Please note that merely _opening_ /dev/snapshot has side effects, so that's
> > not just "three ioctls".
> 
> I search Doc*/power/userland-swsusp.txt, and could not find mentions
> of sideeffects. Perhaps that should be fixed?

Please feel free to make the docs more complete, then.

BTW, if you look at snapshot_open(), you'll immediately see what I mean.

Moreover, the SNAPSHOT_S2RAM ioctl won't execute the suspend notifiers
as appropriate, so that simply doesn't fly.

Rafael

Re: syncing the disks when entering sleep

[Pavel Machek]

On Wed 2010-01-27 21:46:43, Rafael J. Wysocki wrote:
> On Wednesday 27 January 2010, Pavel Machek wrote:
> > 
> > > > Per-system property, which should better be
> > > > per-program-that-requires-suspend. You request suspend without syncing
> > > > (you want it quick, battery is 90%), then the battery runs low, and
> > > > system daeomn requests s2ram, not realizing that someone disabled sync
> > > > from under him.
> > > 
> > > I really prefer a per-system setting.  The program that wants to sync anyway
> > > can easily do that by itself.
> > 
> > Yes, but existing apps do not know they have to sync. You are
> > essentially adding "break back compatibility" system wide option, when
> > better alternative exists... See above for concrete example where it
> > may hurt.
> 
> I don't get what the problem is, really.
> 
> There's _nothing_ here that breaks the existing behavior.  If the user doesn't
> set the switch, everything works as usual.  If he does, breaking the "back
> compatibility" is _his_ problem.

So... you give them option to break back compatibility then blame it
on them.... Yes, we could do it, but it is not good.

And better alternative exists, where you make the option per-suspend
so it does not have chance to break compatibility later.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Sunday 31 January 2010, Pavel Machek wrote:
> On Wed 2010-01-27 21:46:43, Rafael J. Wysocki wrote:
> > On Wednesday 27 January 2010, Pavel Machek wrote:
> > > 
> > > > > Per-system property, which should better be
> > > > > per-program-that-requires-suspend. You request suspend without syncing
> > > > > (you want it quick, battery is 90%), then the battery runs low, and
> > > > > system daeomn requests s2ram, not realizing that someone disabled sync
> > > > > from under him.
> > > > 
> > > > I really prefer a per-system setting.  The program that wants to sync anyway
> > > > can easily do that by itself.
> > > 
> > > Yes, but existing apps do not know they have to sync. You are
> > > essentially adding "break back compatibility" system wide option, when
> > > better alternative exists... See above for concrete example where it
> > > may hurt.
> > 
> > I don't get what the problem is, really.
> > 
> > There's _nothing_ here that breaks the existing behavior.  If the user doesn't
> > set the switch, everything works as usual.  If he does, breaking the "back
> > compatibility" is _his_ problem.
> 
> So... you give them option to break back compatibility then blame it
> on them.... Yes, we could do it, but it is not good.
> 
> And better alternative exists, where you make the option per-suspend
> so it does not have chance to break compatibility later.

I'm not sure what you mean.  Would you like it to reset itself during resume
or something like this?

Rafael

Re: syncing the disks when entering sleep

[Pavel Machek]

> On Sunday 31 January 2010, Pavel Machek wrote:
> > On Wed 2010-01-27 21:46:43, Rafael J. Wysocki wrote:
> > > On Wednesday 27 January 2010, Pavel Machek wrote:
> > > > 
> > > > > > Per-system property, which should better be
> > > > > > per-program-that-requires-suspend. You request suspend without syncing
> > > > > > (you want it quick, battery is 90%), then the battery runs low, and
> > > > > > system daeomn requests s2ram, not realizing that someone disabled sync
> > > > > > from under him.
> > > > > 
> > > > > I really prefer a per-system setting.  The program that wants to sync anyway
> > > > > can easily do that by itself.
> > > > 
> > > > Yes, but existing apps do not know they have to sync. You are
> > > > essentially adding "break back compatibility" system wide option, when
> > > > better alternative exists... See above for concrete example where it
> > > > may hurt.
> > > 
> > > I don't get what the problem is, really.
> > > 
> > > There's _nothing_ here that breaks the existing behavior.  If the user doesn't
> > > set the switch, everything works as usual.  If he does, breaking the "back
> > > compatibility" is _his_ problem.
> > 
> > So... you give them option to break back compatibility then blame it
> > on them.... Yes, we could do it, but it is not good.
> > 
> > And better alternative exists, where you make the option per-suspend
> > so it does not have chance to break compatibility later.
> 
> I'm not sure what you mean.  Would you like it to reset itself during resume
> or something like this?

Yes, that or something similar would certainly help.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Leisner, Martin]

>   -----Original Message-----
>   From: Pavel Machek [mailto:pavel@ucw.cz]
>   Sent: Monday, February 01, 2010 5:49 AM
>   To: Rafael J. Wysocki
>   Cc: Leisner, Martin; linux-pm@lists.linux-foundation.org
>   Subject: Re: [linux-pm] syncing the disks when entering sleep
>   
>   > On Sunday 31 January 2010, Pavel Machek wrote:
>   > > On Wed 2010-01-27 21:46:43, Rafael J. Wysocki wrote:
>   > > > On Wednesday 27 January 2010, Pavel Machek wrote:
>   > > > >
>   > > > > > > Per-system property, which should better be
>   > > > > > > per-program-that-requires-suspend. You request suspend
>   without syncing
>   > > > > > > (you want it quick, battery is 90%), then the battery runs
>   low, and
>   > > > > > > system daeomn requests s2ram, not realizing that someone
>   disabled sync
>   > > > > > > from under him.
>   > > > > >
>   > > > > > I really prefer a per-system setting.  The program that wants
>   to sync anyway
>   > > > > > can easily do that by itself.
>   > > > >
>   > > > > Yes, but existing apps do not know they have to sync. You are
>   > > > > essentially adding "break back compatibility" system wide
>   option, when
>   > > > > better alternative exists... See above for concrete example
>   where it
>   > > > > may hurt.
>   > > >
>   > > > I don't get what the problem is, really.
>   > > >
>   > > > There's _nothing_ here that breaks the existing behavior.  If the
>   user doesn't
>   > > > set the switch, everything works as usual.  If he does, breaking
>   the "back
>   > > > compatibility" is _his_ problem.
>   > >
>   > > So... you give them option to break back compatibility then blame it
>   > > on them.... Yes, we could do it, but it is not good.
>   > >
>   > > And better alternative exists, where you make the option per-suspend
>   > > so it does not have chance to break compatibility later.
>   >
>   > I'm not sure what you mean.  Would you like it to reset itself during
>   resume
>   > or something like this?
>   
>   Yes, that or something similar would certainly help.
>   									Pavel
>   --

So you're asking to give this knob "one shot behavior"
(i.e. "then next sleep won't sync")?

But I'm primarily interested in the behavior on embedded systems (where you control all the processes running -- there's no "user" involved.

If a user starts messing with default settings, any unwanted behavior is the users problem (besides, this should only be writable as root).

marty

Re: syncing the disks when entering sleep

[Nigel Cunningham]

Hi.

Pavel Machek wrote:
>> On Sunday 31 January 2010, Pavel Machek wrote:
>>> On Wed 2010-01-27 21:46:43, Rafael J. Wysocki wrote:
>>>> On Wednesday 27 January 2010, Pavel Machek wrote:
>>>>>>> Per-system property, which should better be
>>>>>>> per-program-that-requires-suspend. You request suspend without syncing
>>>>>>> (you want it quick, battery is 90%), then the battery runs low, and
>>>>>>> system daeomn requests s2ram, not realizing that someone disabled sync
>>>>>>> from under him.
>>>>>> I really prefer a per-system setting.  The program that wants to sync anyway
>>>>>> can easily do that by itself.
>>>>> Yes, but existing apps do not know they have to sync. You are
>>>>> essentially adding "break back compatibility" system wide option, when
>>>>> better alternative exists... See above for concrete example where it
>>>>> may hurt.
>>>> I don't get what the problem is, really.
>>>>
>>>> There's _nothing_ here that breaks the existing behavior.  If the user doesn't
>>>> set the switch, everything works as usual.  If he does, breaking the "back
>>>> compatibility" is _his_ problem.
>>> So... you give them option to break back compatibility then blame it
>>> on them.... Yes, we could do it, but it is not good.
>>>
>>> And better alternative exists, where you make the option per-suspend
>>> so it does not have chance to break compatibility later.
>> I'm not sure what you mean.  Would you like it to reset itself during resume
>> or something like this?
> 
> Yes, that or something similar would certainly help.

No, that would only make things more confusing. What other kernel knob
turns itself off like that? Even drop caches, which might be reasonably
expected to do so, doesn't.

Regards,

Nigel

Re: syncing the disks when entering sleep

[Pavel Machek]

On Tue 2010-02-02 07:55:04, Nigel Cunningham wrote:
> Hi.
> 
> Pavel Machek wrote:
> >> On Sunday 31 January 2010, Pavel Machek wrote:
> >>> On Wed 2010-01-27 21:46:43, Rafael J. Wysocki wrote:
> >>>> On Wednesday 27 January 2010, Pavel Machek wrote:
> >>>>>>> Per-system property, which should better be
> >>>>>>> per-program-that-requires-suspend. You request suspend without syncing
> >>>>>>> (you want it quick, battery is 90%), then the battery runs low, and
> >>>>>>> system daeomn requests s2ram, not realizing that someone disabled sync
> >>>>>>> from under him.
> >>>>>> I really prefer a per-system setting.  The program that wants to sync anyway
> >>>>>> can easily do that by itself.
> >>>>> Yes, but existing apps do not know they have to sync. You are
> >>>>> essentially adding "break back compatibility" system wide option, when
> >>>>> better alternative exists... See above for concrete example where it
> >>>>> may hurt.
> >>>> I don't get what the problem is, really.
> >>>>
> >>>> There's _nothing_ here that breaks the existing behavior.  If the user doesn't
> >>>> set the switch, everything works as usual.  If he does, breaking the "back
> >>>> compatibility" is _his_ problem.
> >>> So... you give them option to break back compatibility then blame it
> >>> on them.... Yes, we could do it, but it is not good.
> >>>
> >>> And better alternative exists, where you make the option per-suspend
> >>> so it does not have chance to break compatibility later.
> >> I'm not sure what you mean.  Would you like it to reset itself during resume
> >> or something like this?
> > 
> > Yes, that or something similar would certainly help.
> 
> No, that would only make things more confusing. What other kernel knob
> turns itself off like that? Even drop caches, which might be reasonably
> expected to do so, doesn't.

IIRC drop caches only drops once...

Fine, that simply shows that we should not have the knob. Rather,
suspend should be getting parameters; one of them is suspend type (S1,
mem, disk), second is test mode (full, only drivers, only core, ...),
next is whether to sync or not...

Maybe we really want sys_suspend() syscall.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Pavel Machek]

> So you're asking to give this knob "one shot behavior"
> (i.e. "then next sleep won't sync")?

Yes.

> But I'm primarily interested in the behavior on embedded systems (where you control all the processes running -- there's no "user" involved.
> 

Well, then "one shot behaviour" does not hurt you, right?

> If a user starts messing with default settings, any unwanted behavior is the users problem (besides, this should only be writable as root).
> 

I'd rather not add traps for the user unless absolutely
neccessary. Not even for root user.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Nigel Cunningham]

Hi.

Pavel Machek wrote:
> On Tue 2010-02-02 07:55:04, Nigel Cunningham wrote:
>> Pavel Machek wrote:
>>>> On Sunday 31 January 2010, Pavel Machek wrote:
>>>>> On Wed 2010-01-27 21:46:43, Rafael J. Wysocki wrote:
>>>>>> On Wednesday 27 January 2010, Pavel Machek wrote:
>>>>>>>>> Per-system property, which should better be
>>>>>>>>> per-program-that-requires-suspend. You request suspend without syncing
>>>>>>>>> (you want it quick, battery is 90%), then the battery runs low, and
>>>>>>>>> system daeomn requests s2ram, not realizing that someone disabled sync
>>>>>>>>> from under him.
>>>>>>>> I really prefer a per-system setting.  The program that wants to sync anyway
>>>>>>>> can easily do that by itself.
>>>>>>> Yes, but existing apps do not know they have to sync. You are
>>>>>>> essentially adding "break back compatibility" system wide option, when
>>>>>>> better alternative exists... See above for concrete example where it
>>>>>>> may hurt.
>>>>>> I don't get what the problem is, really.
>>>>>>
>>>>>> There's _nothing_ here that breaks the existing behavior.  If the user doesn't
>>>>>> set the switch, everything works as usual.  If he does, breaking the "back
>>>>>> compatibility" is _his_ problem.
>>>>> So... you give them option to break back compatibility then blame it
>>>>> on them.... Yes, we could do it, but it is not good.
>>>>>
>>>>> And better alternative exists, where you make the option per-suspend
>>>>> so it does not have chance to break compatibility later.
>>>> I'm not sure what you mean.  Would you like it to reset itself during resume
>>>> or something like this?
>>> Yes, that or something similar would certainly help.
>> No, that would only make things more confusing. What other kernel knob
>> turns itself off like that? Even drop caches, which might be reasonably
>> expected to do so, doesn't.
> 
> IIRC drop caches only drops once...

Yes. It drops them when you set the value non zero (value is a bitflag
saying what to drop). To do the same thing  again, you need to set it to
some other value then to the value you want.

> Fine, that simply shows that we should not have the knob. Rather,
> suspend should be getting parameters; one of them is suspend type (S1,
> mem, disk), second is test mode (full, only drivers, only core, ...),
> next is whether to sync or not...
> 
> Maybe we really want sys_suspend() syscall.

I don't see why it shows that we should not have the knob, or rather,
why you're now arguing that the knob should exist, but be hidden in a
binary only interface. It seems illogical.

Regards,

Nigel

Re: syncing the disks when entering sleep

[Nigel Cunningham]

Hi.

Pavel Machek wrote:
>> So you're asking to give this knob "one shot behavior" (i.e. "then
>> next sleep won't sync")?
> 
> Yes.
> 
>> But I'm primarily interested in the behavior on embedded systems
>> (where you control all the processes running -- there's no "user"
>> involved.
>> 
> 
> Well, then "one shot behaviour" does not hurt you, right?
> 
>> If a user starts messing with default settings, any unwanted
>> behavior is the users problem (besides, this should only be
>> writable as root).
>> 
> 
> I'd rather not add traps for the user unless absolutely neccessary.
> Not even for root user. Pavel

But that's precisely what you're doing. You're advocating making the
behaviour inconsistent. If what you're suggesting is done, you won't be
able to simply cat the sysfs entry to know whether sys_syncing is going
to be done on the next cycle. You'll also have to have knowledge of
whether a cycle has been done since the last time the value is set. The
end result will be someone getting trapped and caught out because they
think '1' in /sys/power/dont_sync (or whatever it's called) means what
it says.

Regards,

Nigel

Re: syncing the disks when entering sleep

[Pavel Machek]

> Hi.
> 
> Pavel Machek wrote:
> >> So you're asking to give this knob "one shot behavior" (i.e. "then
> >> next sleep won't sync")?
> > 
> > Yes.
> > 
> >> But I'm primarily interested in the behavior on embedded systems
> >> (where you control all the processes running -- there's no "user"
> >> involved.
> >> 
> > 
> > Well, then "one shot behaviour" does not hurt you, right?
> > 
> >> If a user starts messing with default settings, any unwanted
> >> behavior is the users problem (besides, this should only be
> >> writable as root).
> >> 
> > 
> > I'd rather not add traps for the user unless absolutely neccessary.
> > Not even for root user. Pavel
> 
> But that's precisely what you're doing. You're advocating making the
> behaviour inconsistent. If what you're suggesting is done, you won't be
> able to simply cat the sysfs entry to know whether sys_syncing is going
> to be done on the next cycle. You'll also have to have knowledge of
> whether a cycle has been done since the last time the value is set. The
> end result will be someone getting trapped and caught out because they
> think '1' in /sys/power/dont_sync (or whatever it's called) means what
> it says.

I'm simply advocating that setting from one suspend should not change
other suspends ... because you have multiple different programs
wanting to suspend the system, all independend. See the example about
system running low on battery earlier in the thread. 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Wednesday 10 February 2010, Pavel Machek wrote:
> > Hi.
> > 
> > Pavel Machek wrote:
> > >> So you're asking to give this knob "one shot behavior" (i.e. "then
> > >> next sleep won't sync")?
> > > 
> > > Yes.
> > > 
> > >> But I'm primarily interested in the behavior on embedded systems
> > >> (where you control all the processes running -- there's no "user"
> > >> involved.
> > >> 
> > > 
> > > Well, then "one shot behaviour" does not hurt you, right?
> > > 
> > >> If a user starts messing with default settings, any unwanted
> > >> behavior is the users problem (besides, this should only be
> > >> writable as root).
> > >> 
> > > 
> > > I'd rather not add traps for the user unless absolutely neccessary.
> > > Not even for root user. Pavel
> > 
> > But that's precisely what you're doing. You're advocating making the
> > behaviour inconsistent. If what you're suggesting is done, you won't be
> > able to simply cat the sysfs entry to know whether sys_syncing is going
> > to be done on the next cycle. You'll also have to have knowledge of
> > whether a cycle has been done since the last time the value is set. The
> > end result will be someone getting trapped and caught out because they
> > think '1' in /sys/power/dont_sync (or whatever it's called) means what
> > it says.
> 
> I'm simply advocating that setting from one suspend should not change
> other suspends ... because you have multiple different programs
> wanting to suspend the system, all independend.

Which is wrong.  There should be one power manager everybody else calls to
suspend the system.  Yes, even if the battery is running critical.

And I guess on the embedded systems in question the situation is exactly like
that.

> See the example about system running low on battery earlier in the thread. 

Which is irrelevant here.

Actually, doing the sync() in the kernel is a hack that we decided not to
remove just because the users space didin't do the right thing on some systems.
If the user space always synced disks before suspending, we wouldn't have to
do that in the kernel.

Same goes for the kernel VT switch, BTW.

So really, I don't see anything wrong with a knob that will turn the kernel
sync off entirely, because that basically means "my user space is not broken".

Rafael

Re: syncing the disks when entering sleep

[Oliver Neukum]

Am Mittwoch, 10. Februar 2010 11:34:23 schrieb Rafael J. Wysocki:
> Actually, doing the sync() in the kernel is a hack that we decided not to
> remove just because the users space didin't do the right thing on some systems.
> If the user space always synced disks before suspending, we wouldn't have to
> do that in the kernel.

If you sync in user space will you not have an inevitable race condition?

	Regards
		Oliver

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Wednesday 10 February 2010, Oliver Neukum wrote:
> Am Mittwoch, 10. Februar 2010 11:34:23 schrieb Rafael J. Wysocki:
> > Actually, doing the sync() in the kernel is a hack that we decided not to
> > remove just because the users space didin't do the right thing on some systems.
> > If the user space always synced disks before suspending, we wouldn't have to
> > do that in the kernel.
> 
> If you sync in user space will you not have an inevitable race condition?

Do you mean the race with applications that write to disks while the sustem is
suspending?

Rafael

Re: syncing the disks when entering sleep

[Oliver Neukum]

Am Mittwoch, 10. Februar 2010 11:58:22 schrieb Rafael J. Wysocki:
> On Wednesday 10 February 2010, Oliver Neukum wrote:
> > Am Mittwoch, 10. Februar 2010 11:34:23 schrieb Rafael J. Wysocki:
> > > Actually, doing the sync() in the kernel is a hack that we decided not to
> > > remove just because the users space didin't do the right thing on some systems.
> > > If the user space always synced disks before suspending, we wouldn't have to
> > > do that in the kernel.
> > 
> > If you sync in user space will you not have an inevitable race condition?
> 
> Do you mean the race with applications that write to disks while the sustem is
> suspending?

Yes.

	Regards
		Oliver

Re: syncing the disks when entering sleep

[Pavel Machek]

> On Wednesday 10 February 2010, Pavel Machek wrote:
> > > Hi.
> > > 
> > > Pavel Machek wrote:
> > > >> So you're asking to give this knob "one shot behavior" (i.e. "then
> > > >> next sleep won't sync")?
> > > > 
> > > > Yes.
> > > > 
> > > >> But I'm primarily interested in the behavior on embedded systems
> > > >> (where you control all the processes running -- there's no "user"
> > > >> involved.
> > > >> 
> > > > 
> > > > Well, then "one shot behaviour" does not hurt you, right?
> > > > 
> > > >> If a user starts messing with default settings, any unwanted
> > > >> behavior is the users problem (besides, this should only be
> > > >> writable as root).
> > > >> 
> > > > 
> > > > I'd rather not add traps for the user unless absolutely neccessary.
> > > > Not even for root user. Pavel
> > > 
> > > But that's precisely what you're doing. You're advocating making the
> > > behaviour inconsistent. If what you're suggesting is done, you won't be
> > > able to simply cat the sysfs entry to know whether sys_syncing is going
> > > to be done on the next cycle. You'll also have to have knowledge of
> > > whether a cycle has been done since the last time the value is set. The
> > > end result will be someone getting trapped and caught out because they
> > > think '1' in /sys/power/dont_sync (or whatever it's called) means what
> > > it says.
> > 
> > I'm simply advocating that setting from one suspend should not change
> > other suspends ... because you have multiple different programs
> > wanting to suspend the system, all independend.
> 
> Which is wrong.  There should be one power manager everybody else calls to
> suspend the system.  Yes, even if the battery is running critical.

Really? If so then it is misdesigned.

Before the "don't sync" proposal, it was okay to have multiple
power managers.

Actually I have three on zaurus. There's in-kernel suspend on battery
critical, then there's somehing userspace in desktop environment, and
then I'm triggering suspends by hand using echo.

> So really, I don't see anything wrong with a knob that will turn the kernel
> sync off entirely, because that basically means "my user space is
> not broken".

Because, very easily, parts of my users space may be broken. So knob
is wrong to be system-wide.

       	   	     	     	  	    	      	   	 Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Wednesday 10 February 2010, Oliver Neukum wrote:
> Am Mittwoch, 10. Februar 2010 11:58:22 schrieb Rafael J. Wysocki:
> > On Wednesday 10 February 2010, Oliver Neukum wrote:
> > > Am Mittwoch, 10. Februar 2010 11:34:23 schrieb Rafael J. Wysocki:
> > > > Actually, doing the sync() in the kernel is a hack that we decided not to
> > > > remove just because the users space didin't do the right thing on some systems.
> > > > If the user space always synced disks before suspending, we wouldn't have to
> > > > do that in the kernel.
> > > 
> > > If you sync in user space will you not have an inevitable race condition?
> > 
> > Do you mean the race with applications that write to disks while the sustem is
> > suspending?
> 
> Yes.

That would matter if sync meant mandatory flushing all data to the storage
medium, but it doesn't.  From a filesystem point of view it's an advisory
thing, something like "I wish you flushed buffers right now".

Rafael

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Wednesday 10 February 2010, Pavel Machek wrote:
> > On Wednesday 10 February 2010, Pavel Machek wrote:
> > > > Hi.
> > > > 
> > > > Pavel Machek wrote:
> > > > >> So you're asking to give this knob "one shot behavior" (i.e. "then
> > > > >> next sleep won't sync")?
> > > > > 
> > > > > Yes.
> > > > > 
> > > > >> But I'm primarily interested in the behavior on embedded systems
> > > > >> (where you control all the processes running -- there's no "user"
> > > > >> involved.
> > > > >> 
> > > > > 
> > > > > Well, then "one shot behaviour" does not hurt you, right?
> > > > > 
> > > > >> If a user starts messing with default settings, any unwanted
> > > > >> behavior is the users problem (besides, this should only be
> > > > >> writable as root).
> > > > >> 
> > > > > 
> > > > > I'd rather not add traps for the user unless absolutely neccessary.
> > > > > Not even for root user. Pavel
> > > > 
> > > > But that's precisely what you're doing. You're advocating making the
> > > > behaviour inconsistent. If what you're suggesting is done, you won't be
> > > > able to simply cat the sysfs entry to know whether sys_syncing is going
> > > > to be done on the next cycle. You'll also have to have knowledge of
> > > > whether a cycle has been done since the last time the value is set. The
> > > > end result will be someone getting trapped and caught out because they
> > > > think '1' in /sys/power/dont_sync (or whatever it's called) means what
> > > > it says.
> > > 
> > > I'm simply advocating that setting from one suspend should not change
> > > other suspends ... because you have multiple different programs
> > > wanting to suspend the system, all independend.
> > 
> > Which is wrong.  There should be one power manager everybody else calls to
> > suspend the system.  Yes, even if the battery is running critical.
> 
> Really? If so then it is misdesigned.
> 
> Before the "don't sync" proposal, it was okay to have multiple
> power managers.
> 
> Actually I have three on zaurus. There's in-kernel suspend on battery
> critical, then there's somehing userspace in desktop environment, and
> then I'm triggering suspends by hand using echo.

To be precise, 1 and 3 are things that override the power manager.

And if you override the power manager, you're supposed to know what you're
doing, aren't you?

> > So really, I don't see anything wrong with a knob that will turn the kernel
> > sync off entirely, because that basically means "my user space is
> > not broken".
> 
> Because, very easily, parts of my users space may be broken.

How exactly would they be broken?

Why do you want to protect the users from themselves for what it's worth?

Why don't we just assume that the user who sets the knob knows what he's doing?

Rafael

Re: syncing the disks when entering sleep

[Leisner, Martin]

>   -----Original Message-----
>   From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
>   Sent: Wednesday, February 10, 2010 5:34 AM
>   To: linux-pm@lists.linux-foundation.org
>   Cc: Pavel Machek; Nigel Cunningham; Leisner, Martin
>   Subject: Re: [linux-pm] syncing the disks when entering sleep
>   
>   On Wednesday 10 February 2010, Pavel Machek wrote:
>   > > Hi.
>   > >
>   > > Pavel Machek wrote:
>   > > >> So you're asking to give this knob "one shot behavior" (i.e.
>   "then
>   > > >> next sleep won't sync")?
>   > > >
>   > > > Yes.
>   > > >
>   > > >> But I'm primarily interested in the behavior on embedded systems
>   > > >> (where you control all the processes running -- there's no "user"
>   > > >> involved.
>   > > >>
>   > > >
>   > > > Well, then "one shot behaviour" does not hurt you, right?
>   > > >
>   > > >> If a user starts messing with default settings, any unwanted
>   > > >> behavior is the users problem (besides, this should only be
>   > > >> writable as root).
>   > > >>
>   > > >
>   > > > I'd rather not add traps for the user unless absolutely
>   neccessary.
>   > > > Not even for root user. Pavel
>   > >
>   > > But that's precisely what you're doing. You're advocating making the
>   > > behaviour inconsistent. If what you're suggesting is done, you won't
>   be
>   > > able to simply cat the sysfs entry to know whether sys_syncing is
>   going
>   > > to be done on the next cycle. You'll also have to have knowledge of
>   > > whether a cycle has been done since the last time the value is set.
>   The
>   > > end result will be someone getting trapped and caught out because
>   they
>   > > think '1' in /sys/power/dont_sync (or whatever it's called) means
>   what
>   > > it says.
>   >
>   > I'm simply advocating that setting from one suspend should not change
>   > other suspends ... because you have multiple different programs
>   > wanting to suspend the system, all independend.
>   
>   Which is wrong.  There should be one power manager everybody else calls
>   to
>   suspend the system.  Yes, even if the battery is running critical.
>   
>   And I guess on the embedded systems in question the situation is exactly
>   like
>   that.
>   
>   > See the example about system running low on battery earlier in the
>   thread.
>   
>   Which is irrelevant here.
>   
>   Actually, doing the sync() in the kernel is a hack that we decided not
>   to
>   remove just because the users space didin't do the right thing on some
>   systems.
>   If the user space always synced disks before suspending, we wouldn't
>   have to
>   do that in the kernel.
>   
>   Same goes for the kernel VT switch, BTW.
>   
>   So really, I don't see anything wrong with a knob that will turn the
>   kernel
>   sync off entirely, because that basically means "my user space is not
>   broken".
>   
>   Rafael

This is interesting.   When I saw the sync's I said to myself "WTF?"
It definitely should be a knob because it implements policy behavior -- 
which is often detrimental to power saving efforts.

marty

Re: syncing the disks when entering sleep

[Leisner, Martin]

>   -----Original Message-----
>   From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
>   Sent: Wednesday, February 10, 2010 2:19 PM
>   To: Pavel Machek
>   Cc: linux-pm@lists.linux-foundation.org; Nigel Cunningham; Leisner,
>   Martin
>   Subject: Re: [linux-pm] syncing the disks when entering sleep
>   
......
>   > > So really, I don't see anything wrong with a knob that will turn the
>   kernel
>   > > sync off entirely, because that basically means "my user space is
>   > > not broken".
>   >
>   > Because, very easily, parts of my users space may be broken.
>   
>   How exactly would they be broken?
>   
>   Why do you want to protect the users from themselves for what it's
>   worth?
>   
>   Why don't we just assume that the user who sets the knob knows what he's
>   doing?
>   
>   Rafael

Exactly what I proposed -- if people who are root start playing with kernel settings, they'll get defined, documented behavior.   If they don't like it,
let them not play.   I see no reason to idiot proof root.

The problem is if the disk is not spinning, the system is in laptop mode, and sleep is entered, it makes no sense to spin up the disk to sync it in
embedded systems (i.e. the vendor controls all the software).

If root does
   dd if=/dev/zero of=/dev/sda
and they trash their disk, so be it.

marty

Re: syncing the disks when entering sleep

[Pavel Machek]

Hi!

> ......
> >   > > So really, I don't see anything wrong with a knob that will turn the
> >   kernel
> >   > > sync off entirely, because that basically means "my user space is
> >   > > not broken".
> >   >
> >   > Because, very easily, parts of my users space may be broken.
> >   
> >   How exactly would they be broken?
> >   
> >   Why do you want to protect the users from themselves for what it's
> >   worth?
> >   
> >   Why don't we just assume that the user who sets the knob knows what he's
> >   doing?
> 
> Exactly what I proposed -- if people who are root start playing with kernel settings, they'll get defined, documented behavior.   If they don't like it,
> let them not play.   I see no reason to idiot proof root.
>

And I see no reason to have bad interface when we can have good one.

You don't have to be idiot to do "echo disabled > sync; echo mem >
state" and then forget to undo the sync setting when it resumes
20hours later.
 
> The problem is if the disk is not spinning, the system is in laptop mode, and sleep is entered, it makes no sense to spin up the disk to sync it in
> embedded systems (i.e. the vendor controls all the software).
> 

I agree that functionality would be nice. I don't agree with proposed interface.



-- 

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Pavel Machek]

> > Really? If so then it is misdesigned.
> > 
> > Before the "don't sync" proposal, it was okay to have multiple
> > power managers.
> > 
> > Actually I have three on zaurus. There's in-kernel suspend on battery
> > critical, then there's somehing userspace in desktop environment, and
> > then I'm triggering suspends by hand using echo.
> 
> To be precise, 1 and 3 are things that override the power manager.
> 
> And if you override the power manager, you're supposed to know what you're
> doing, aren't you?

I know what I'm doing, but I'd prefer traps not being set for me.

> > > So really, I don't see anything wrong with a knob that will turn the kernel
> > > sync off entirely, because that basically means "my user space is
> > > not broken".
> > 
> > Because, very easily, parts of my users space may be broken.
> 
> How exactly would they be broken?

I have 3 power managers. You called that broken before.

How is power manager expected to work on zaurus, which suspends from
kernel on battery critical? echo no > sync; sync; echo mem > state;
echo yes > sync?

Its still racy..

> Why don't we just assume that the user who sets the knob knows what he's doing?
>

Because better alternatives exist. Like 'echo mem:nosync > state'.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Thursday 11 February 2010, Pavel Machek wrote:
> 
> > > Really? If so then it is misdesigned.
> > > 
> > > Before the "don't sync" proposal, it was okay to have multiple
> > > power managers.
> > > 
> > > Actually I have three on zaurus. There's in-kernel suspend on battery
> > > critical, then there's somehing userspace in desktop environment, and
> > > then I'm triggering suspends by hand using echo.
> > 
> > To be precise, 1 and 3 are things that override the power manager.
> > 
> > And if you override the power manager, you're supposed to know what you're
> > doing, aren't you?
> 
> I know what I'm doing, but I'd prefer traps not being set for me.

What are you talking about?  Either you set the knob or you don't.  If you
don't, nothing changes.

> > > > So really, I don't see anything wrong with a knob that will turn the kernel
> > > > sync off entirely, because that basically means "my user space is
> > > > not broken".
> > > 
> > > Because, very easily, parts of my users space may be broken.
> > 
> > How exactly would they be broken?
> 
> I have 3 power managers.

You don't.

> You called that broken before.

Because it is so.  Think about user space suspend hooks, for example, like
s2ram.  Surely your kernel emergency suspend can't use anything like this?

> How is power manager expected to work on zaurus, which suspends from
> kernel on battery critical? echo no > sync; sync; echo mem > state;
> echo yes > sync?
> 
> Its still racy..

Please refer to my reply to Oliver.

> > Why don't we just assume that the user who sets the knob knows what he's doing?
> >
> 
> Because better alternatives exist. Like 'echo mem:nosync > state'.

That is a sledgehammer.

I really don't understand your objections here and none of the "alternatives"
you've been talking about so far would be acceptable to me.

Rafael

Re: syncing the disks when entering sleep

[Nigel Cunningham]

Hi.

Rafael J. Wysocki wrote:
> On Thursday 11 February 2010, Pavel Machek wrote:
>>>> Really? If so then it is misdesigned.
>>>>
>>>> Before the "don't sync" proposal, it was okay to have multiple
>>>> power managers.
>>>>
>>>> Actually I have three on zaurus. There's in-kernel suspend on battery
>>>> critical, then there's somehing userspace in desktop environment, and
>>>> then I'm triggering suspends by hand using echo.
>>> To be precise, 1 and 3 are things that override the power manager.
>>>
>>> And if you override the power manager, you're supposed to know what you're
>>> doing, aren't you?
>> I know what I'm doing, but I'd prefer traps not being set for me.
> 
> What are you talking about?  Either you set the knob or you don't.  If you
> don't, nothing changes.
> 
>>>>> So really, I don't see anything wrong with a knob that will turn the kernel
>>>>> sync off entirely, because that basically means "my user space is
>>>>> not broken".
>>>> Because, very easily, parts of my users space may be broken.
>>> How exactly would they be broken?
>> I have 3 power managers.
> 
> You don't.
> 
>> You called that broken before.
> 
> Because it is so.  Think about user space suspend hooks, for example, like
> s2ram.  Surely your kernel emergency suspend can't use anything like this?
> 
>> How is power manager expected to work on zaurus, which suspends from
>> kernel on battery critical? echo no > sync; sync; echo mem > state;
>> echo yes > sync?
>>
>> Its still racy..
> 
> Please refer to my reply to Oliver.
> 
>>> Why don't we just assume that the user who sets the knob knows what he's doing?
>>>
>> Because better alternatives exist. Like 'echo mem:nosync > state'.
> 
> That is a sledgehammer.
> 
> I really don't understand your objections here and none of the "alternatives"
> you've been talking about so far would be acceptable to me.

I'm in violent agreement with Rafael. Pavel, I think you're overruled on
this one.

Regards,

Nigel

Re: syncing the disks when entering sleep

[Matthew Garrett]

On Wed, Feb 10, 2010 at 07:42:43PM +0100, Rafael J. Wysocki wrote:

> That would matter if sync meant mandatory flushing all data to the storage
> medium, but it doesn't.  From a filesystem point of view it's an advisory
> thing, something like "I wish you flushed buffers right now".

It's defined as forcing actual writing on Linux. It doesn't guarantee 
that it'll be consistent (eg, XFS may flush to the journal rather than 
the filesystem), but it should mean that all cached data is on disk in 
some form.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: syncing the disks when entering sleep

[Pavel Machek]

> > > > > Actually, doing the sync() in the kernel is a hack that we decided not to
> > > > > remove just because the users space didin't do the right thing on some systems.
> > > > > If the user space always synced disks before suspending, we wouldn't have to
> > > > > do that in the kernel.
> > > > 
> > > > If you sync in user space will you not have an inevitable race condition?
> > > 
> > > Do you mean the race with applications that write to disks while the sustem is
> > > suspending?
> > 
> > Yes.
> 
> That would matter if sync meant mandatory flushing all data to the storage
> medium, but it doesn't.  From a filesystem point of view it's an advisory
> thing, something like "I wish you flushed buffers right now".

No, I don't think so. Maybe posix states its advisory and only starts
a writeback, but linux historicaly did the right thing.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

document open(/dev/snapshot) sideeffects -- was Re: [linux-pm] syncing the disks when entering sleep

[Pavel Machek]

Hi!

> > > Please note that merely _opening_ /dev/snapshot has side effects, so that's
> > > not just "three ioctls".
> > 
> > I search Doc*/power/userland-swsusp.txt, and could not find mentions
> > of sideeffects. Perhaps that should be fixed?
> 
> Please feel free to make the docs more complete, then.

Yes, something like this?

Signed-off-by: Pavel Machek <pavel@ucw.cz>

--- a/Documentation/power/userland-swsusp.txt
+++ b/Documentation/power/userland-swsusp.txt
@@ -24,6 +24,10 @@ assumed to be in the resume mode.  The device cannot be open for simultaneous
 reading and writing.  It is also impossible to have the device open more than
 once at a time.
 
+Even opening the device has side effects. Data structures are
+allocated, and PM_HIBERNATION_PREPARE / PM_RESTORE_PREPARE chains are
+called.
+
 The ioctl() commands recognized by the device are:
 
 SNAPSHOT_FREEZE - freeze user space processes (the current process is



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

document open(/dev/snapshot) sideeffects -- was Re: syncing the disks when entering sleep

[Pavel Machek]

Hi!

> > > Please note that merely _opening_ /dev/snapshot has side effects, so that's
> > > not just "three ioctls".
> > 
> > I search Doc*/power/userland-swsusp.txt, and could not find mentions
> > of sideeffects. Perhaps that should be fixed?
> 
> Please feel free to make the docs more complete, then.

Yes, something like this?

Signed-off-by: Pavel Machek <pavel@ucw.cz>

--- a/Documentation/power/userland-swsusp.txt
+++ b/Documentation/power/userland-swsusp.txt
@@ -24,6 +24,10 @@ assumed to be in the resume mode.  The device cannot be open for simultaneous
 reading and writing.  It is also impossible to have the device open more than
 once at a time.
 
+Even opening the device has side effects. Data structures are
+allocated, and PM_HIBERNATION_PREPARE / PM_RESTORE_PREPARE chains are
+called.
+
 The ioctl() commands recognized by the device are:
 
 SNAPSHOT_FREEZE - freeze user space processes (the current process is



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: document open(/dev/snapshot) sideeffects -- was Re: [linux-pm] syncing the disks when entering sleep

[Rafael J. Wysocki]

On Monday 05 April 2010, Pavel Machek wrote:
> Hi!
> 
> > > > Please note that merely _opening_ /dev/snapshot has side effects, so that's
> > > > not just "three ioctls".
> > > 
> > > I search Doc*/power/userland-swsusp.txt, and could not find mentions
> > > of sideeffects. Perhaps that should be fixed?
> > 
> > Please feel free to make the docs more complete, then.
> 
> Yes, something like this?

Yes, looks reasonable.

I've applied it to suspend-2.6/linux-next (added a changelog).

Rafael


> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> --- a/Documentation/power/userland-swsusp.txt
> +++ b/Documentation/power/userland-swsusp.txt
> @@ -24,6 +24,10 @@ assumed to be in the resume mode.  The device cannot be open for simultaneous
>  reading and writing.  It is also impossible to have the device open more than
>  once at a time.
>  
> +Even opening the device has side effects. Data structures are
> +allocated, and PM_HIBERNATION_PREPARE / PM_RESTORE_PREPARE chains are
> +called.
> +
>  The ioctl() commands recognized by the device are:
>  
>  SNAPSHOT_FREEZE - freeze user space processes (the current process is
> 
> 
> 
> 

Re: document open(/dev/snapshot) sideeffects -- was Re: syncing the disks when entering sleep

[Rafael J. Wysocki]

On Monday 05 April 2010, Pavel Machek wrote:
> Hi!
> 
> > > > Please note that merely _opening_ /dev/snapshot has side effects, so that's
> > > > not just "three ioctls".
> > > 
> > > I search Doc*/power/userland-swsusp.txt, and could not find mentions
> > > of sideeffects. Perhaps that should be fixed?
> > 
> > Please feel free to make the docs more complete, then.
> 
> Yes, something like this?

Yes, looks reasonable.

I've applied it to suspend-2.6/linux-next (added a changelog).

Rafael


> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> --- a/Documentation/power/userland-swsusp.txt
> +++ b/Documentation/power/userland-swsusp.txt
> @@ -24,6 +24,10 @@ assumed to be in the resume mode.  The device cannot be open for simultaneous
>  reading and writing.  It is also impossible to have the device open more than
>  once at a time.
>  
> +Even opening the device has side effects. Data structures are
> +allocated, and PM_HIBERNATION_PREPARE / PM_RESTORE_PREPARE chains are
> +called.
> +
>  The ioctl() commands recognized by the device are:
>  
>  SNAPSHOT_FREEZE - freeze user space processes (the current process is
> 
> 
> 
> 

Re: document open(/dev/snapshot) sideeffects -- was Re: [linux-pm] syncing the disks when entering sleep

[Pavel Machek]

On Fri 2010-04-23 20:28:55, Rafael J. Wysocki wrote:
> On Monday 05 April 2010, Pavel Machek wrote:
> > Hi!
> > 
> > > > > Please note that merely _opening_ /dev/snapshot has side effects, so that's
> > > > > not just "three ioctls".
> > > > 
> > > > I search Doc*/power/userland-swsusp.txt, and could not find mentions
> > > > of sideeffects. Perhaps that should be fixed?
> > > 
> > > Please feel free to make the docs more complete, then.
> > 
> > Yes, something like this?
> 
> Yes, looks reasonable.
> 
> I've applied it to suspend-2.6/linux-next (added a changelog).

Thanks!
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: document open(/dev/snapshot) sideeffects -- was Re: syncing the disks when entering sleep

[Pavel Machek]

On Fri 2010-04-23 20:28:55, Rafael J. Wysocki wrote:
> On Monday 05 April 2010, Pavel Machek wrote:
> > Hi!
> > 
> > > > > Please note that merely _opening_ /dev/snapshot has side effects, so that's
> > > > > not just "three ioctls".
> > > > 
> > > > I search Doc*/power/userland-swsusp.txt, and could not find mentions
> > > > of sideeffects. Perhaps that should be fixed?
> > > 
> > > Please feel free to make the docs more complete, then.
> > 
> > Yes, something like this?
> 
> Yes, looks reasonable.
> 
> I've applied it to suspend-2.6/linux-next (added a changelog).

Thanks!
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html