rickysarraf / laptop-mode-tools

Power Savings tool for Linux
https://www.researchut.com/tags/laptop-mode-tools/
GNU General Public License v2.0
550 stars 47 forks source link

Fix restoring filesystem commit interval #189

Open maciejsszmigiero opened 1 year ago

maciejsszmigiero commented 1 year ago

grep without "-E" option uses basic regular expressions, where "+" repetition operator needs to be backslashed as "+", but replace_numeric_mount_option() function in the laptop-mode module used just a plain "+".

This resulted in this function being unable to find a commit interval option in the saved mount options and so substituting it with the default "commit=0" instead.

Due to this mistake any non-default commit interval would get reset to the filesystem-specific default value (as provided by "commit=0") when laptop mode was deactivated (like when the AC supply was plugged).

rickysarraf commented 1 year ago

I can't seem to reproduce this issue you describe.

do you have the following enabled /etc/laptop-mode/laptop-mode.conf ?

ENABLE_LAPTOP_MODE_ON_AC LM_BATT_MAX_LOST_WORK_SECONDS LM_AC_MAX_LOST_WORK_SECONDS

maciejsszmigiero commented 1 year ago

The minimal test case is probably just calling this grep directly:

$ echo ",rw,noatime,commit=60," | grep ",commit=[0123456789]+,"

versus:

$ echo ",rw,noatime,commit=60," | grep ",commit=[0123456789]\+,"
,rw,noatime,commit=60,

For reference:

$ grep --version
grep (GNU grep) 3.8
Copyright (C) 2022 Free Software Foundation, Inc.

I don't have ENABLE_LAPTOP_MODE_ON_AC enabled, but I do have ENABLE_LAPTOP_MODE_ON_BATTERY enabled and LM_BATT_MAX_LOST_WORK_SECONDS left at the default 600.

The issue happens in the following scenario:

  1. Have LMT-supported filesystem mounted with non-default commit interval (like commit=60),
  2. Unplug the AC adapter,
  3. LMT will activate and remount the filesystem with commit interval equal to LM_BATT_MAX_LOST_WORK_SECONDS (like 600 in my case),
  4. Plug the AC adapter once again,
  5. LMT will de-activate but due to this bug it won't restore the previous commit=60 value, but rather overwrite it with commit=0 (the kernel default one).