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

linux-stracciatella: add 'CONFIG_MODULE_UNLOAD=y' #803

Closed
gbyl opened this issue Dec 25, 2023 · 14 comments
Closed

linux-stracciatella: add 'CONFIG_MODULE_UNLOAD=y' #803

gbyl opened this issue Dec 25, 2023 · 14 comments
Labels
bug Something isn't working packages Add or improve packages of the repository

Comments

@gbyl
Copy link

gbyl commented Dec 25, 2023

Device/Package information:

  • Device: reMarkable 1
  • OS version: 2.15.1.1189
  • Stock Kernel version: Linux reMarkable 5.4.70-v1.2.5-rm10x #⁠1
  • Stracciatella Kernel version: Linux reMarkable 5.4.70 #⁠2
  • Package: linux-stracciatella

Describe the issue
The stock rM1 kernel does not allow unloading modules.

Packages, such as KOReader, that rely on modprobe brcmfmac and modprobe -r brcmfmac to enable and disable Wi-Fi (and to detect Wi-Fi status) do not work as expected.

Workaround

  1. Blacklist the Broadcom FullMAC WLAN driver modules /etc/modprobe.d/blacklist.conf
blacklist brcmfmac
blacklist brcmutil
blacklist cfg80211
  1. Disable DHCP (and other services) to prevent startup delays and conflicts
systemctl disable dhcpcd disable-wowlan manual-sync wpa_supplicant
  1. Modify Wi-Fi enable script /opt/koreader/enable-wifi.sh
#!/bin/sh
if ! lsmod | grep -q brcmfmac; then
  modprobe brcmfmac
  sleep 1
fi

ip link set wlan0 up
systemctl start dhcpcd
wpa_supplicant -B -i wlan0 -C /var/run/wpa_supplicant
  1. Modify Wi-Fi disable script /opt/koreader/disable-wifi.sh
#!/bin/sh
wpa_cli terminate
systemctl stop dhcpcd
ip link set wlan0 down
modprobe -r brcmfmac 2> /dev/null

Side Benefits
With this workaround I am seeing an increase in battery life, and USBnet startup of around 2 seconds

Questions

  1. Does the rM2 kernel allow unloading modules?
  2. @Etn40ff Are there any disadvantages to adding CONFIG_MODULE_UNLOAD=y to linux-stracciatella?
@gbyl gbyl added bug Something isn't working packages Add or improve packages of the repository labels Dec 25, 2023
@Eeems
Copy link
Member

Eeems commented Dec 25, 2023

Thank you for reporting this compatibility issue. Is there a reason you haven't opened up an upstream issue/PR to link to this one?

Packages, such as KOReader, that rely on modprobe brcmfmac and modprobe -r brcmfmac to enable and disable Wi-Fi (and to detect Wi-Fi status) do not work as expected.

xochitl also tries to unload/load the wifi module, same with rm2-suspend-fix.

Questions

1. Does the rM2 kernel allow unloading modules?

Yes it does, therwise xochitl wouldn't be able to unload the wifi module.

3. Disable DHCP (and other services) to prevent startup delays and conflicts

systemctl disable dhcpcd disable-wowlan manual-sync wpa_supplicant

wpa_supplicant should already be disabled as xochitl manually starts the service. Disabling things like manual-sync and dhcpcd are of concern, as they are required for things like usb and syncing to work.

The other changes to the koreader suspend/resume scripts would better live in rm2-suspend-fix after thinking through how to do it in a way that is compatible with all the other packages in toltec. Oxide would not be happy with it for example.

@gbyl
Copy link
Author

gbyl commented Dec 26, 2023

Is there a reason you haven't opened up an upstream issue/PR to link to this one?

Do you mean in reMarkable/linux? I don't see a lot of activity there and I don't have the kernel dev knowledge to justify this change. So, I thought the best chance to see this even considered would be in Etn40ff's kernel.

Glad to hear it works on rM2 at least. In rM1 I get the output below. Are you (or someone else) able to confirm?

$ modprobe -r brcmfmac
modprobe: ERROR: could not remove 'brcmfmac': Function not implemented
$ rmmod brcmfmac
rmmod: ERROR: Module unloading is not supported

Good point about the disabling of some of those services. I think usb is using busybox-ifplugd and busybox-udhcpd. So, I haven't seen any issues when dhcpcd is off. But I haven't tested this extensively either.

@Eeems
Copy link
Member

Eeems commented Dec 26, 2023

Is there a reason you haven't opened up an upstream issue/PR to link to this one?

Do you mean in reMarkable/linux? I don't see a lot of activity there and I don't have the kernel dev knowledge to justify this change. So, I thought the best chance to see this even considered would be in Etn40ff's kernel.

No, I meant Etn40ff's repository for the kernel, which is upstream for toltec.

Glad to hear it works on rM2 at least. In rM1 I get the output below. Are you (or someone else) able to confirm?

I only have a rM1 myself, so I can't confirm that things work as expected with this kernel on a rM2.

Good point about the disabling of some of those services. I think usb is using busybox-ifplugd and busybox-udhcpd. So, I haven't seen any issues when dhcpcd is off. But I haven't tested this extensively either.

Oh, you are probably correct about the USB handling, it's still required for normal wireless communication to work when other applications are running. For toltec to accept a fix, we would want to avoid outright disabling services where possible, and try to resolve any conflicts instead. Which is why manual-sync.service exists, we disable the built-in sync.service that xochtil requires, as it has a hard link on xochitl.service. This causes conflicts for launchers. So instead we add the manual-sync.service which removes the hard link, and will play nice with launchers.

# sync.service interferes with launchers
# we use manual-sync.service instead
if [[ "x$(systemctl is-enabled sync.service)" != "xmasked" ]]; then
systemctl mask sync.service
fi
if ! is-active manual-sync.service; then
systemctl enable --now manual-sync.service
fi

Upon reviewing things, disabling manual-sync.service on startup would be fine, as it gets started by the xochitl startup script. The script also reenables it, so it's not really worth disabling it without also changing this.

if ! systemctl is-active --quiet manual-sync.service; then
systemctl enable --now manual-sync.service
fi

On the rM1 we disable sys-subsystem-net-devices-usb1.device to avoid slowdown on startup caused by the networking waiting on usb1, which isn't ever going to be a valid network device.

if [[ "$arch" == "rm1" ]] && ! is-masked sys-subsystem-net-devices-usb1.device; then
echo "Disabling usb1 network device to avoid long boots"
systemctl mask sys-subsystem-net-devices-usb1.device

Upon looking at disable-wowlan.service, I would not recommend disabling it either, as it's intentionally disabling the wake on lan feature of the wireless card. Could you provide some information on what lead you to believe that this is slowing down startup?
image

@Etn40ff
Copy link

Etn40ff commented Dec 26, 2023

I can confirm that CONFIG_MODULE_UNLOAD is not enabled for rm1 but it is enabled for rm2. I have no idea why they did so upstream. I do not see any problem in enabling it also for rm1 so I will as soon as I have 5 minutes to play with this. I will not be able to sets on a rm1 though so I will need your help.

@Eeems
Copy link
Member

Eeems commented Dec 26, 2023

I can confirm that CONFIG_MODULE_UNLOAD is not enabled for rm1 but it is enabled for rm2. I have no idea why they did so upstream. I do not see any problem in enabling it also for rm1 so I will as soon as I have 5 minutes to play with this. I will not be able to sets on a rm1 though so I will need your help.

They probably did it to allow unloading the wifi module on the rM2 when it suspends, which I don't believe they ever backported to the rM1.

@gbyl
Copy link
Author

gbyl commented Dec 26, 2023

@Etn40ff Thank you so much! You can count on me for rM1 testing. I would submit a PR to your repo, but I am not sure if other files need to change. I also couldn't figure out how you are generating the tar file that is sourced by the toltec recipe. I see rM2 has

CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MODVERSIONS=y
CONFIG_MODULE_SRCVERSION_ALL=y

https://github.com/Etn40ff/linux-remarkable/blob/c908b16d6b848964ecc9b116a024f247c290a1bf/arch/arm/configs/zero-sugar_defconfig#L44-L47

whereas rM1 only has

CONFIG_MODULES=y

https://github.com/Etn40ff/linux-remarkable/blob/c908b16d6b848964ecc9b116a024f247c290a1bf/arch/arm/configs/zero-gravitas_defconfig#L36

@Eeems
Copy link
Member

Eeems commented Dec 26, 2023

I would submit a PR to your repo, but I am not sure if other files need to change. I also couldn't figure out how you are generating the tar file that is sourced by the toltec recipe.

It's just using a GitHub feature that lets you download an export of any commit hash. So if you have your own fork with changes you can change the URL to be your fork, and the commit hash of the latest commit you made. Then you can use toltecmk to build the recipe locally for testing.

@gbyl
Copy link
Author

gbyl commented Dec 26, 2023

Thanks for clearing that up Eeems!

Also, I was wondering where that manual-sync.service came from. Your explanation makes perfect sense.

To answer your questions, when brcmfmac is not loaded, dhcpcd.service restarts 5 times and remains in an errored state. Then it needs to be manually restarted for Wi-Fi to work.

dhcpcd[138]: wlan0: interface not found or invalid
dhcpcd[138]: dhcpcd exited
systemd[1]: dhcpcd.service: Control process exited, code=exited, status=1/FAILURE
systemd[1]: dhcpcd.service: Failed with result 'exit-code'.
systemd[1]: Failed to start dhcpcd on all interfaces.
systemd[1]: dhcpcd.service: Scheduled restart job, restart counter is at 1.

When brcmfmac is not loaded, disable-wowlan.service adds a substantial delay. For instance, if I try to connect with ssh -v -o "ConnectionAttempts 45" [email protected] it only succeeds at the last attempt.

systemd[1]: sys-subsystem-net-devices-wlan0.device: Job sys-subsystem-net-devices-wlan0.device/start timed out.
systemd[1]: Timed out waiting for device /sys/subsystem/net/devices/wlan0.
systemd[1]: Dependency failed for Disable wowlan.
systemd[1]: disable-wowlan.service: Job disable-wowlan.service/start failed with result 'dependency'.
systemd[1]: sys-subsystem-net-devices-wlan0.device: Job sys-subsystem-net-devices-wlan0.device/start failed with result 'timeout'.

On the rM1 we disable sys-subsystem-net-devices-usb1.device to avoid slowdown on startup caused by the networking waiting on usb1, which isn't ever going to be a valid network device.

That's interesting! Based on this, I managed to speed up USBnet startup even further by disabling [email protected], finally avoiding the errors below -- which remained even with the rM1 solution you mentioned.

systemd[1]: [email protected]: Bound to unit sys-subsystem-net-devices-usb1.device, but unit isn't active.
systemd[1]: Dependency failed for ifplugd on usb1.
systemd[1]: [email protected]: Job [email protected]/start failed with result 'dependency'.

@Eeems
Copy link
Member

Eeems commented Dec 26, 2023

To answer your questions, when brcmfmac is not loaded, dhcpcd.service restarts 5 times and remains in an errored state. Then it needs to be manually restarted for Wi-Fi to work.

dhcpcd[138]: wlan0: interface not found or invalid
dhcpcd[138]: dhcpcd exited
systemd[1]: dhcpcd.service: Control process exited, code=exited, status=1/FAILURE
systemd[1]: dhcpcd.service: Failed with result 'exit-code'.
systemd[1]: Failed to start dhcpcd on all interfaces.
systemd[1]: dhcpcd.service: Scheduled restart job, restart counter is at 1.

When brcmfmac is not loaded, disable-wowlan.service adds a substantial delay. For instance, if I try to connect with ssh -v -o "ConnectionAttempts 45" [email protected] it only succeeds at the last attempt.

systemd[1]: sys-subsystem-net-devices-wlan0.device: Job sys-subsystem-net-devices-wlan0.device/start timed out.
systemd[1]: Timed out waiting for device /sys/subsystem/net/devices/wlan0.
systemd[1]: Dependency failed for Disable wowlan.
systemd[1]: disable-wowlan.service: Job disable-wowlan.service/start failed with result 'dependency'.
systemd[1]: sys-subsystem-net-devices-wlan0.device: Job sys-subsystem-net-devices-wlan0.device/start failed with result 'timeout'.

Based on this, I think it would make sense to leave the modules loaded on boot, and only unload it on suspend or disable of wifi. A script can determine if wifi should be on or not by looking at the value stored under [General] with the key wifion in /home/root/.config/remarkable/xochitl.conf.

We would have to think through all the possible launchers and making sure they properly unloaded the module if it should be off, and that they work properly with enabling the module when turning on wifi.

The current list of launchers we would have to work with:

  • draft
    • Doesn't manage wifi or suspend
    • There is no wrapper script, but an ExecStartPre= could be added to the systemd service.
    • If xochitl and koreader are updated, they would be fine with being launched from draft and the module being disabled, and everything
  • xochitl
    • Adding unloading based on wifi state would be easy enough to add to the wrapper script
    • Doesn't enable/disable the module on rM1, and skips systemd suspend management, so maybe I should get around to releasing this preload
  • oxide
    • Uses systemd for suspend, a rm1-suspend-fix package like the rm2 one would work here
    • Handles unloading/loading wifi drivers on suspend/resume, but should have an issue opened upstream to make it only load the modules if wifi is enabled
  • remux
    • Same notes as for oxide
  • koreader

On the rM1 we disable sys-subsystem-net-devices-usb1.device to avoid slowdown on startup caused by the networking waiting on usb1, which isn't ever going to be a valid network device.

That's interesting! Based on this, I managed to speed up USBnet startup even further by disabling [email protected], finally avoiding the errors below -- which remained even with the rM1 solution you mentioned.

systemd[1]: [email protected]: Bound to unit sys-subsystem-net-devices-usb1.device, but unit isn't active.
systemd[1]: Dependency failed for ifplugd on usb1.
systemd[1]: [email protected]: Job [email protected]/start failed with result 'dependency'.

Oh, we should probably add that to toltec-base, could you open a PR to add it?

@Etn40ff
Copy link

Etn40ff commented Dec 27, 2023

I propose to remove

CONFIG_MODVERSIONS=y
CONFIG_MODULE_SRCVERSION_ALL=y

for rm2 and add

CONFIG_MODULE_UNLOAD=y

for rm1.

If you agree I will make the appropriate changes.

@Eeems
Copy link
Member

Eeems commented Dec 27, 2023

I would recommend leaving CONFIG_MODVERSIONS=y, otherwise you risk allowing people to load kernel modules in that will crash.
https://www.kernel.org/doc/html/latest/kbuild/modules.html#module-versioning

@Eeems
Copy link
Member

Eeems commented Dec 27, 2023

Is there a reason you haven't opened up an upstream issue/PR to link to this one?

Do you mean in reMarkable/linux? I don't see a lot of activity there and I don't have the kernel dev knowledge to justify this change. So, I thought the best chance to see this even considered would be in Etn40ff's kernel.

No, I meant Etn40ff's repository for the kernel, which is upstream for toltec.

Upon thinking about this a bit more, it would also be worth upstreaming this all the way to rM's kernel. While there isn't a bunch of activity on the repo, that's because this is just their public facing repository. They do all their development in private repos, and then update the public repos after releasing OS updates. They have been receptive to changes in the past. It can tend to be slow though as any work on merging community member changes on public repos appears to only be done as a side-of-the-desk activity.

@Etn40ff
Copy link

Etn40ff commented Dec 28, 2023

This is now #808, it needs testing on RM1

@gbyl
Copy link
Author

gbyl commented Jan 8, 2024

Closed by Etn40ff/linux-remarkable#2 and #808. Thank you Etn40ff and Eeems!

@gbyl gbyl closed this as completed Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working packages Add or improve packages of the repository
Projects
None yet
Development

No branches or pull requests

3 participants