fix do reboot on oom and lower vm min free to 4M on 64M hardware #151

fwiessner · 2021-07-05T15:00:30+02:00

fwiessner commented

2021-07-05 15:00:30 +02:00

First-time contributor

This patch sets reboot on OOM as default to ensure we never have half-working nodes and lowers the OOM limit to 4M on 64M hardware.

fwiessner added 1 commit 2021-07-05 15:00:31 +02:00

d6a4185d17 fix do reboot on oom and lower vm min free to 4M on 64M hardware

fwiessner referenced this pull request

2021-07-05 17:08:40 +02:00

oom-killer macht keinen reboot - limit für oom-killer nicht optimal #131

adschm commented

2021-07-05 22:42:33 +02:00

package/base-files/files/etc/sysctl.d/20-oom_reboot.conf

Sowas können wir auch selber pflegen, das muss nicht in OpenWrt gepatcht werden.

Ansonsten habe ich zwecks der minfree Limits nachgefragt und folgende Antwort erhalten:
http://lists.openwrt.org/pipermail/openwrt-devel/2021-February/034062.html

> package/base-files/files/etc/sysctl.d/20-oom_reboot.conf Sowas können wir auch selber pflegen, das muss nicht in OpenWrt gepatcht werden. Ansonsten habe ich zwecks der minfree Limits nachgefragt und folgende Antwort erhalten: http://lists.openwrt.org/pipermail/openwrt-devel/2021-February/034062.html

fwiessner commented

2021-07-06 00:58:02 +02:00

First-time contributor

Ich kann zwar verstehen, warum das gemacht wird, allerdings ist es meiner Meinung nach nicht sinnvoll. Im Prinzip ist es doch egal ob der node wegen OOM Killer kaputt geht aber noch bridgen/routen kann, oder ob er panict weil er zum bridgen/routen nix mehr allocaten kann.

fwiessner commented

2021-07-06 00:59:48 +02:00

First-time contributor

package/base-files/files/etc/sysctl.d/20-oom_reboot.conf

Sowas können wir auch selber pflegen, das muss nicht in OpenWrt gepatcht werden.

Ich habe fblaese gefragt wie und wo, damit war er zufrieden. Ansonsten kann ich es auch einfach sein lassen, oder man möge mir mitteilen wo es den besser aufgehoben wäre.

> > package/base-files/files/etc/sysctl.d/20-oom_reboot.conf > > Sowas können wir auch selber pflegen, das muss nicht in OpenWrt gepatcht werden. > Ich habe fblaese gefragt wie und wo, damit war er zufrieden. Ansonsten kann ich es auch einfach sein lassen, oder man möge mir mitteilen wo es den besser aufgehoben wäre.

fbl commented

2021-07-06 10:04:08 +02:00

Ich habe fblaese gefragt wie und wo, damit war er zufrieden. Ansonsten kann ich es auch einfach sein lassen, oder man möge mir mitteilen wo es den besser aufgehoben wäre.

Ich zitiere einfach nochmal meine Antwort: "ich denke wir sollten das nicht über openwrt-patches umsetzen, sondern lieber passende sysctl.conf anlegen. ich bin mir nur noch nicht sicher wo, deshalb hab ich bisher noch nichts gebaut"

Ich bin ebenfalls der Meinung, dass das nicht als OpenWrt Patch umgesetzt werden sollte, da das erhöhten Pflegeaufwand bedeutet und die Änderung weniger nachvollziehbar macht. Bei min_free_kbytes kann man darüber noch diskutieren, damit die Unterscheidung nach Arbeitsspeichergröße weiterhin erfolgen kann.

Ansonsten habe ich zwecks der minfree Limits nachgefragt und folgende Antwort erhalten:
http://lists.openwrt.org/pipermail/openwrt-devel/2021-February/034062.html

Danke fürs nachfragen. Ich bin davon ausgegangen, dass es dafür irgendeinen Grund geben wird. Ich würde das Speicherlimit nicht verändern, solange wir damit nicht tatsächlich ein Problem lösen können. Und da bei 64MB RAM (ohne 5 GHz) normalerweise knapp die Hälfte frei ist sieht mir das eher nach einem Memleak aus und wird mit 4MB zusätzlichem RAM auch nicht gelöst.

panic_on_oom hingegen sollten wir in jedem Fall einbauen. Ich frage mich, warum das bei OpenWrt nicht Standard ist, denn da läuft ja normalerweise quasi nichts nicht-essentielles; der Router ist nach OOM Killer auf jeden Fall auf die ein oder andere Art kaputt.

> Ich habe fblaese gefragt wie und wo, damit war er zufrieden. Ansonsten kann ich es auch einfach sein lassen, oder man möge mir mitteilen wo es den besser aufgehoben wäre. Ich zitiere einfach nochmal meine Antwort: "ich denke wir sollten das nicht über openwrt-patches umsetzen, sondern lieber passende sysctl.conf anlegen. ich bin mir nur noch nicht sicher wo, deshalb hab ich bisher noch nichts gebaut" Ich bin ebenfalls der Meinung, dass das nicht als OpenWrt Patch umgesetzt werden sollte, da das erhöhten Pflegeaufwand bedeutet und die Änderung weniger nachvollziehbar macht. Bei `min_free_kbytes` kann man darüber noch diskutieren, damit die Unterscheidung nach Arbeitsspeichergröße weiterhin erfolgen kann. ----- > Ansonsten habe ich zwecks der minfree Limits nachgefragt und folgende Antwort erhalten: http://lists.openwrt.org/pipermail/openwrt-devel/2021-February/034062.html Danke fürs nachfragen. Ich bin davon ausgegangen, dass es dafür irgendeinen Grund geben wird. Ich würde das Speicherlimit nicht verändern, solange wir damit nicht tatsächlich ein Problem lösen können. Und da bei 64MB RAM (ohne 5 GHz) normalerweise knapp die Hälfte frei ist sieht mir das eher nach einem Memleak aus und wird mit 4MB zusätzlichem RAM auch nicht gelöst. `panic_on_oom` hingegen sollten wir in jedem Fall einbauen. Ich frage mich, warum das bei OpenWrt nicht Standard ist, denn da läuft ja normalerweise quasi nichts nicht-essentielles; der Router ist nach OOM Killer auf jeden Fall auf die ein oder andere Art kaputt.

fwiessner commented

2021-07-06 10:12:50 +02:00

First-time contributor

"ich denke wir sollten das nicht über openwrt-patches umsetzen, sondern lieber passende sysctl.conf anlegen" - genau das macht doch package/base-files/files/etc/sysctl.d/20-oom_reboot.conf - oder was sonst muss man patchen damit das file beim build erzeugt wird?

fbl commented

2021-07-06 10:15:23 +02:00

Ah, jetzt verstehe ich, woher die Verwirrung kommt. Da hätte ich genauer sein können, sorry.

Wir versuchen unsere Änderungen - soweit irgendwie möglich - in OpenWrt Packages zu halten, und möglichst wenig bis nichts am OpenWrt zu verändern. Die findest du in src/packages/fff. Dadurch werden Änderungen nachvollziehbarer und Modularisierung möglich.

Ich bin mir nur wie gesagt selbst noch nicht ganz sicher, in welches Paket die Änderungen am besten passen.

Ah, jetzt verstehe ich, woher die Verwirrung kommt. Da hätte ich genauer sein können, sorry. Wir versuchen unsere Änderungen - soweit irgendwie möglich - in OpenWrt Packages zu halten, und möglichst wenig bis nichts am OpenWrt zu verändern. Die findest du in src/packages/fff. Dadurch werden Änderungen nachvollziehbarer und Modularisierung möglich. Ich bin mir nur wie gesagt selbst noch nicht ganz sicher, in welches Paket die Änderungen am besten passen.

adschm commented

2021-07-06 22:57:59 +02:00

Ich habe das schon vor ein paar Monaten mal mit einem Archer C60 getestet (64MB RAM und ath10k). Der bleibt (im Rahmen dessen, was man "messen" kann) immer gleich instabil, egal ob man da 2, 4, 6 oder 8 MiB reserviert.

Der Grund, warum restart nicht automatisch durchgeführt wird, ist derselbe, warum man das generell nicht gerne macht: Man verliert so die Gelegenheit, sich auf dem "kaputten" Gerät einzuloggen. Im schlimmsten Fall (z.B. 32 MB RAM) komme ich so ggf. nie mehr in einen Zustand, in dem ich das Gerät überhaupt verwenden kann, und habe effektiv eine Reboot-Loop.

Was man für den konkreten Fall unserer Firmware vorzieht ist aber sicherlich eine Design-Entscheidung, ich glaube nicht das hier ein Ansatz "richtiger" ist als der andere.

Für mich ist ein Gerät, das sich alle paar Tage wegen OOM neu starten muss, aber ohnehin "kaputt" und sollte durch ein ordentliches ersetzt werden.

Ich habe das schon vor ein paar Monaten mal mit einem Archer C60 getestet (64MB RAM und ath10k). Der bleibt (im Rahmen dessen, was man "messen" kann) immer gleich instabil, egal ob man da 2, 4, 6 oder 8 MiB reserviert. Der Grund, warum restart nicht automatisch durchgeführt wird, ist derselbe, warum man das generell nicht gerne macht: Man verliert so die Gelegenheit, sich auf dem "kaputten" Gerät einzuloggen. Im schlimmsten Fall (z.B. 32 MB RAM) komme ich so ggf. nie mehr in einen Zustand, in dem ich das Gerät überhaupt verwenden kann, und habe effektiv eine Reboot-Loop. Was man für den konkreten Fall unserer Firmware vorzieht ist aber sicherlich eine Design-Entscheidung, ich glaube nicht das hier ein Ansatz "richtiger" ist als der andere. Für mich ist ein Gerät, das sich alle paar Tage wegen OOM neu starten muss, aber ohnehin "kaputt" und sollte durch ein ordentliches ersetzt werden.

fwiessner commented

2021-07-07 15:27:18 +02:00

First-time contributor

Der Grund, warum restart nicht automatisch durchgeführt wird, ist derselbe, warum man das generell nicht gerne macht: Man verliert so die Gelegenheit, sich auf dem "kaputten" Gerät einzuloggen. Im schlimmsten Fall (z.B. 32 MB RAM) komme ich so ggf. nie mehr in einen Zustand, in dem ich das Gerät überhaupt verwenden kann, und habe effektiv eine Reboot-Loop.

Auf Systemen die produktiv laufen ist es mir lieber, das System heilt sich selbst durch reboot und panic und macht ein notify, als dass man irgendwohin anfahren muss. Logs kann man per udp remote sysloggen, das ist also kein Argument. Wenn sshd durch OOM stirbt kommt man auch nicht mehr drauf und kann auch nix "sehen", also auch kein Argument.

Was man für den konkreten Fall unserer Firmware vorzieht ist aber sicherlich eine Design-Entscheidung, ich glaube nicht das hier ein Ansatz "richtiger" ist als der andere.

Für mich ist ein Gerät, das sich alle paar Tage wegen OOM neu starten muss, aber ohnehin "kaputt" und sollte durch ein ordentliches ersetzt werden.

Das ist Quatsch. Es ist ja kein Hardwareproblem. Das Problem tritt irgendwie seit der 2021er Firmware auf. Mir wäre es lieber die Ursache zu finden und zu beheben, anstatt einfach zu sagen man braucht mehr RAM oder andere HW.

> Der Grund, warum restart nicht automatisch durchgeführt wird, ist derselbe, warum man das generell nicht gerne macht: Man verliert so die Gelegenheit, sich auf dem "kaputten" Gerät einzuloggen. Im schlimmsten Fall (z.B. 32 MB RAM) komme ich so ggf. nie mehr in einen Zustand, in dem ich das Gerät überhaupt verwenden kann, und habe effektiv eine Reboot-Loop. Auf Systemen die produktiv laufen ist es mir lieber, das System heilt sich selbst durch reboot und panic und macht ein notify, als dass man irgendwohin anfahren muss. Logs kann man per udp remote sysloggen, das ist also kein Argument. Wenn sshd durch OOM stirbt kommt man auch nicht mehr drauf und kann auch nix "sehen", also auch kein Argument. > > Was man für den konkreten Fall unserer Firmware vorzieht ist aber sicherlich eine Design-Entscheidung, ich glaube nicht das hier ein Ansatz "richtiger" ist als der andere. > > Für mich ist ein Gerät, das sich alle paar Tage wegen OOM neu starten muss, aber ohnehin "kaputt" und sollte durch ein ordentliches ersetzt werden. Das ist Quatsch. Es ist ja kein Hardwareproblem. Das Problem tritt irgendwie seit der 2021er Firmware auf. Mir wäre es lieber die Ursache zu finden und zu beheben, anstatt einfach zu sagen man braucht mehr RAM oder andere HW.

fwiessner added 1 commit 2021-07-31 00:20:56 +02:00

f0b03b48d4 Merge branch 'master' into master

fbl commented

2021-12-30 15:58:38 +01:00

Mit #195 (dort wird die sysctl Änderung in einem Package gemacht, statt als build_patch) obsolet.

Dennoch bleibt hier möglicherweise das Problem, dass irgendetwas Speicher leaked. Falls dem so ist, bitte noch mal separat ein Issue dazu anlegen.

Mit #195 (dort wird die sysctl Änderung in einem Package gemacht, statt als build_patch) obsolet. Dennoch bleibt hier möglicherweise das Problem, dass irgendetwas Speicher leaked. Falls dem so ist, bitte noch mal separat ein Issue dazu anlegen.

fbl closed this pull request

2021-12-30 15:58:38 +01:00

fbl added the

rejected

label 2021-12-30 15:58:53 +01:00