Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix restoring filesystem commit interval #189

Open
wants to merge 1 commit into
base: lmt-upstream
Choose a base branch
from

Conversation

maciejsszmigiero
Copy link
Contributor

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

grep without "-E" option uses basic regular expressions, where "+"
repetition operator needs to be backslashed as "\+", but
replace_numeric_mount_option() function 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
Copy link
Owner

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

@rickysarraf rickysarraf self-assigned this Sep 29, 2022
@maciejsszmigiero
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants