[intrepid] laptop-mode-tools needs to change its default settings to match acpi-support and add hooks for pm-utils

Bug #250935 reported by Alexey Borzenkov
30
This bug affects 1 person
Affects Status Importance Assigned to Milestone
laptop-mode-tools (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

Binary package hint: laptop-mode-tools

Currently acpi-support is running laptop-mode incorrectly. First of all it executes laptop-mode with start and stop commands directly, which interferes with settings users have in /etc/laptop-mode/laptop-mode.conf. Secondly, immediately after that it applies hard-coded power management and spindown time values, which interfere with the same settings specified in /etc/laptop-mode/laptop-mode.conf.

Please keep in mind, that acpi-support applies these settings only when laptop-mode is enabled, so it is natural for laptop-mode to handle these settings by itself, which it is better at, that is. To do this I propose these steps:

1) Change default laptop-mode settings to match those of acpi-support: this way users without custom configs won't notice any difference.
2) Since pm-utils is handling power states and hibernate/thaw/sleep/resume add pm-utils hooks to handle laptop-mode reloading
3) Change acpi-support not to interfere with laptop-mode, since now it is handled by pm-utils

The attached debdiff does steps 1 and 2. For step 3 I will be filing acpi-support bug-report.

Revision history for this message
Alexey Borzenkov (snaury) wrote :
Revision history for this message
tricky1 (tricky1) wrote :

I just wondered what part of the software of an 'avarage/ standard' Ubuntu installation triggers disk acess somwhere in between 30 and 90 seconds?

In case that there is no other foreground activity (e.g. the user is away and the screensaver blanked the display) an apporoach delaying non urgent disk acess would _vastly_ diminish energy consumption.

Revision history for this message
Alexey Borzenkov (snaury) wrote :

tricky1, what does this have to do with my bug report?

Note, that the values you see in my debdiff are the ones currently specified in /etc/default/acpi-support and /etc/acpi/power.sh, nothing more. /etc/default/acpi-support currently specifies SPINDOWN_TIME=12, but 12 is not seconds, it's a magical value that translates to 60 seconds (because in that lower range it's a number of 5 seconds intervals). It is not my job to argue correctness of default values, all I want is to move hard-coded values from acpi-support to laptop-mode-tools where they can be clearly and easily tweaked by users (i.e. 60 seconds is much more elaborate than magical number 12).

I wonder if this would be triaged and/or considered by developers before Intrepid freeze...

Revision history for this message
Tore Anderson (toreanderson) wrote :

I'm a bit off-topic here, but seeing how this bug is about moving functionality from acpi-support to laptop-mode, I'm wondering if you know where pm-utils fits in?

That package also does a bit of toggling power saving functions when switching between mains/battery. I recently submitted a patch for SATA ALPM, too. However I have no idea if there actually is a strategy to gather up all of those scripts in pm-utils or laptop-mode (and eventually deprecate the other) or if they're supposed to be (arbitrarily) distributed between them. It seems a rather chaotic at the moment, to be frank. I wonder if there's any official "master plan" here...

Anyway, I hope your patch makes it in (as well as the one for acpi-support). But in order for it to work properly /etc/default/acpi-support also needs to be updated to ENABLE_LAPTOP_MODE=true which really should be the default - the machines that get "odd hangs" should be blacklisted instead of ruining battery runtime for everyone...

That setting is good example of the chaos I mentioned above, by the way... You'll need to enable functionality provided by package A which is controlled by package B in a configuration file for package C - yikes!

Well, that turned halfway into a rant - apologies. Anyway. If there's any Ubuntu devs listening, please do apply this patch and the one in #250938 before Intrepid, as the way the three power management packages interact now is clearly broken. And hopefully there'll be some strategy to clean up the power management soon. The current state of things is just depressing. If somebody can point out the overall direction I'd love to help out with patches - I'd love to see Ubuntu being able to squeeze the maximum uptime possible out of my battery. :-)

Tore

Revision history for this message
Tormod Volden (tormodvolden) wrote :

Alexey, thanks for working on a debdiff. I have some comments:
1. I think the package should install default hooks in /usr/lib/pm-utils. The /etc directory is for system-wide configuration by the local administrator.
2. Does these fixes make sense in Debian? If so please file them there also.
3. Are some fixes needed in the pm-utils package as well? Can you reference other bug reports (with patches)?
4. Does the laptop_mode script handle "auto" correctly? (I don't remember)

Thanks again for looking thoroughly into this, it really is time we get it fixed.

Revision history for this message
Alexey Borzenkov (snaury) wrote :

1. Well, if it's true it should be very easy to change: just s[etc/pm/][usr/lib/pm-utils/] in the debdiff.
2. Unfortunately, I have no clue if Debian currently uses acpi-support/pm-utils or anything else. At the very least they don't have hacks in /etc/acpi/power.sh like Ubuntu has, so they must be doing something differently.
3. At first I was thinking about changing pm-utils, but then I realized that pm-utils shouldn't control laptop-mode-tools either. So I decided that the correct place for hooks is laptop-mode-tools itself.
4. As far as I reviewed laptop_mode script it should. It should actually do it without arguments as well, but I thought it is better to pass auto anyway. It's just important not to pass start/stop, comment "Old options. We always do "auto" for any option now" is misleading: check for start/stop is done after auto-detection.

Additionally, compared to Debian, Ubuntu's laptop_mode has state check commented out, which means we don't have problems like "previous state is incorrect after hibernate/thaw and settings are not reapplied". Just calling laptop_mode auto at the right moments seems to be sufficient in Ubuntu (as long as /etc/init.d/laptop-mode is in runlevels).

Revision history for this message
Tormod Volden (tormodvolden) wrote :

Upload sponsors, please see bug #250938 for the related change in acpi-support.

Changed in laptop-mode-tools:
status: New → Confirmed
Revision history for this message
Tormod Volden (tormodvolden) wrote :

Alexey, I added your patch from this bug to the merge in bug #255446. I haven't fully tested the merge yet, and maybe it need more changes (or actually drop some of the Ubuntu delta). Please test it if you can.

Revision history for this message
ceg (ceg) wrote :

Hi Tormod,
you asked:

3. Are some fixes needed in the pm-utils package as well?
Can you reference other bug reports (with patches)?

I do remeber two things, first Bug #239419 (pm-utils)
  "pm-utils has laptop-tools script which conflicts with laptop-mode-tools"
has been wrongly set as a duplicate of this bug, but is actually messing with laptop-mode-tools job.

IMHO the cleanest (and non misleading) fix would be to just remove the /usr/lib/pm-utils/power.d/laptop-tools script from pm-tools. (laptop-mode-tools does all that in a configurable way)

And as Alexey has pointed out in a comment, as this script has (and currently is) setting /proc/sys/vm/laptop_mode unconditionaly. So it is tested is may be time now to enable laptop-mode by default with package laptop-mode-tools. (It will still only activate laptop-mode on battery by default.)

But then finally Bug #59695 (High frequency of load/unload cycles on some hard disks may shorten lifetime) can be fixed. Simply by enabling CONTROL_HD_POWERMGMT=1 in laptop-mode.conf.
The NOLM_AC_HD_POWERMGMT=254 setting that is present will then disable head parking by default.

Revision history for this message
Alexey Borzenkov (snaury) wrote :

ceg, that's why this bug was marked as intrepid. Intrepid doesn't have none of that in pm-utils.

Revision history for this message
Tormod Volden (tormodvolden) wrote :

In principle we concentrate on getting things right in the next release, Intrepid. If possible (time and technical constraints) we can backport the fixes to Hardy. But please make all patches and discussion against Intrepid, otherwise we'll get nowhere.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package laptop-mode-tools - 1.45-1ubuntu1

---------------
laptop-mode-tools (1.45-1ubuntu1) intrepid; urgency=low

  [ Tormod Volden ]
  * Merge from debian unstable (LP: #255446), remaining changes:
    - etc/init.d/laptop-mode: Check if laptop mode is disabled in
      /etc/default/laptop-mode (that's the file laptop_mode looks into, too,
      but it incorrectly evaluates the ENABLE_LAPTOP_MODE setting) as well
      as in /etc/default/acpi-support.
    - usr/sbin/laptop_mode: Do not parse arguments, ignore "force" and
      let start/stop mean enable/disable.
    - /usr/sbin/laptop_mode: Do not read $ACTIVATE_WITH_POSSIBLE_DATA_LOSS
      from /var/run/laptop-mode-state to see if state has changed.
    - debian/rules: Do not ship acpi/apm scripts (we handle that ourselves)
    - debian/laptop-mode-tools.preinst: Remove any old acpi/apm scripts
    - debian/laptop-mode-tools.postinst: Create /etc/default/laptop-mode
      (with ENABLE_LAPTOP_MODE=false) if the file does not exist yet and we
      are on ppc.
    - Also accept "stop" as second argument since the init script
      calls laptop_mode with "init stop"

  [ Alexey Borzenkov ]
  * Change default settings to match acpi-support (LP: #250935)
    - etc/laptop-mode/laptop-mode.conf: change default spindown time and
      power management modes
  * Add pm-utils hooks to ensure laptop-mode is running as expected after
    power change or thaw/resume
    - power.d/laptop-mode: runs laptop-mode on power change
    - sleep.d/96laptop-mode: runs laptop-mode on thaw/resume

laptop-mode-tools (1.45-1) unstable; urgency=low

  * New upstream version 1.45.
  * New upstream version 1.44:
    * Strip non-numeric suffixes from kernel minor version number.
      Closes: #488950.
    * Remove spurious error message on machines that don't have
      /sys/class/power_supply. Closes: #490167.
    * Fix incorrect path to laptop-mode.conf in the laptop-mode.conf(8)
      manual page. Closes: #488261.
    * Add sched-mc-power-savings module. Closes: #490587.

laptop-mode-tools (1.43-1) unstable; urgency=low

  * New upstream version 1.43:
    * Don't use temp file in init script, that doesn't work if /tmp is
      mounted readonly. Closes: #480946.
    * Fix IPW2100 power management so that power management mode stays
      enabled even on AC. If we don't do this, we can't control the
      transmit power at all because we let the chipset handle it.
      Closes: #481180.
    * Replace *_ENABLE_HAL_POLLING settings by *_DISABLE_HAL_POLLING.
      More intuitive that way. The old settings were also interpreted
      in reverse. The backward compatibility layer actually *fixes that*
      because it was a bug, so behaviour will change for 1.42
      installations. Closes: #482307.
    * Get rid of bashism in laptop-mode module.
  * Put URL on separate line in README. Closes: #486467.
  * Set DM-Upload-Allowed field to yes.
  * Fix bashism in lm-profiler script. Closes: #480606

 -- Tormod Volden <email address hidden> Wed, 06 Aug 2008 20:06:08 +0200

Changed in laptop-mode-tools:
status: Confirmed → Fix Released
Revision history for this message
ceg (ceg) wrote :

Thank you for your comments.

Then the current state I see in intrepid is this:

- You still need to "ENABLE_LAPTOP_MODE=1" in /etc/default/acpi-support (Bug 244838)

- The /etc/laptop-mode/laptop-mode.conf default hdparm -B value has been changed from 254 to 255.

This may be OK for consistency of the current state, but 254 is actually needed for many laptop harddisks to stop load cycling. (hdparm -B255 turns off the disk's apm feature, but this only turns off the spin down timer in many disks and doesn't increase the head parking timer at all, which is the issue here.)
(Added that info to https://wiki.ubuntu.com/PowerManagement)

Laptop-mode has always shipped with the sane defaults like 254. I see no real reason to keep suboptimal defaults from the ubuntu hack. The defaults can just be merged from the debian package I guess.

Revision history for this message
Tormod Volden (tormodvolden) wrote :

> You still need to "ENABLE_LAPTOP_MODE=1" in /etc/default/acpi-support (Bug 244838)

It seems simple enough to just remove that check from laptop-mode-tools. But I am a little bit concerned about upgrades - what should we do - just turn on laptop-mode for everyone, which defaulted to off before? Should we delete that line from people's configuration files? Or comment it?

> The /etc/laptop-mode/laptop-mode.conf default hdparm -B value has been changed from 254 to 255.

This should have been 254 IIRC, since bug #172282 got fixed with laptop-mode-tools (1.35-1ubuntu1). Is it really back to 255?

Revision history for this message
Tormod Volden (tormodvolden) wrote :

> This should have been 254 IIRC, since bug #172282 got fixed with laptop-mode-tools (1.35-1ubuntu1). Is it really back to 255?

Yes, indeed. The 255 was reintroduced by the patch in this bug report, in order to keep the exact same functionality as before in Ubuntu, when it was set by acpi-support (and not by laptop-mode-tools).

But I guess it will make sense to move one step forward and set it to 254, as recommended upstream. The best would be to reopen bug #172282, which has pointers to the relevant discussion. Looking at it, there are actually some systems where 255 is better, but I think 254 is optimal for the majority.

Revision history for this message
Alexey Borzenkov (snaury) wrote :

Tormod, ceg, I thought one step at a time is always much easier to accept, that's why I chose to migrate old settings first. I feared otherwise people would end up arguing what defaults are better/worse and this would get nowhere. Now there are just two steps, indeed. Figuring out best default values (maybe previous values were indeed better? who knows?), then dumping need for ENABLE_LAPTOP_MODE=true altogether: laptop-mode-tools can behave on its own, thank you. :) That's what I see in Debian sid right now, if my eyes are not deceiving me. If there are any concerns about system freezes or any issues like that, you could try setting ENABLE_LAPTOP_MODE_ON_BATTERY=0 by default. This way laptop_mode won't set /proc/sys/vm/laptop_mode to possibly troubling values, but would still do a lot of useful things (i.e. all those NOLM settings).

Revision history for this message
Tormod Volden (tormodvolden) wrote :

Alexey, I totally agree, doing it like you did was the best way. Change code first, preserving behaviour, and then flip the configuration settings later. I just forgot about that setting being in this patch.

We must be careful to avoid regressions. laptop-mode-tools used to be an add-on package, later become installed by default, but disabled. for a reason or many. But maybe these reasons are not valid any longer. Anyway, changing power and disk settings is always controversial.

About ENABLE_LAPTOP_MODE, I commented on this in bug #244838 - I think that's the right place for that discussion. We have spread things out in way too many bug reports maybe :)

One difference with Debian is that there, people install the package because they want it, and uninstall it if it doesn't work well. On Ubuntu, we ship it to everybody, and it must Just Work.

Revision history for this message
ceg (ceg) wrote :

Alexey, you did good in making the patch acceptable. (And since you had success, even better!)

So second step would be to flip the settings the right way.

hdparm -B254 needs to be reintroduced because bug #172282 has changed that setting in the meantime.

(Yann who reported 254 did not work for him might very well have suffered from /etc/acpi/power.sh messing around. I happen to have a WD drive and have experienced exactly this.)

Revision history for this message
Dana Goyette (danagoyette) wrote :

Why is this change still here? This seems like some sort of breakage in the logic.
    - usr/sbin/laptop_mode: Do not parse arguments, ignore "force" and
      let start/stop mean enable/disable.

Revision history for this message
Tormod Volden (tormodvolden) wrote :

Dana, the change is still there because nobody has had time to change it and test it carefully enough to suggest a package update. It's probably too late for Intrepid, but let's hope for Jaunty. The earlier it gets fixed in the development cycle, the better.

The package got a lot of improvements early in Intrepid, mainly because people here came up with patches and were actively testing. Then it got quiet, so I hope that means it works pretty ok by now.

Revision history for this message
Colin Watson (cjwatson) wrote :

I've put 'hdparm -B 254' back now:

laptop-mode-tools (1.45-1ubuntu2) intrepid; urgency=low

  * etc/laptop-mode/laptop-mode.conf: Go back to 'hdparm -B 254';
    acpi-support has been fixed to do that now, so let's not have
    laptop-mode-tools undo the effectiveness of that fix in the name of
    consistency with an old version (LP: #172282).

 -- Colin Watson <email address hidden> Wed, 08 Oct 2008 15:05:10 +0100

Revision history for this message
Valentin Neacsu (valentin.neacsu) wrote :

Laptop mode doesn't seem to sense state changes when resuming from standby or hibernate. Should I open a new bug for this?

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.