syslogd(8) add a protection against OOM killer.
AbandonedPublic
Actions

Authored by araujo on Jan 18 2016, 6:35 AM.

Details

Reviewers

• rodrigc
bapt
imp

Group Reviewers

manpages

Summary

If the system exhausts available memory and swap, it starts to kill processes and report killed processes with syslog facility. syslogd(8) should not be killed or else system administrator has not complete information about the case. Practice shows that syslogd may be killed like any other process.

Let's teach syslogd to protect itself from OOM killer. The following patch introduces new command line option "-O" and system administrator is allowed to have a line in its /etc/rc.conf:

PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=204741
syslogd_flags="-O"

In this case, syslogd became protected.

Test Plan

Create a VM with 256M of ram.
Run syslogd with -O.
Run anything else that will consume memory.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2226
Build 2235: arc lint + arc unit

Event Timeline

araujo updated this revision to Diff 12405.Jan 18 2016, 6:35 AM

araujo retitled this revision from to syslogd(8) add a protection against OOM killer..

araujo updated this object.

araujo edited the test plan for this revision. (Show Details)

araujo added reviewers: • rodrigc, bapt.

Herald added a reviewer: manpages. · View Herald TranscriptJan 18 2016, 6:35 AM

Herald added a subscriber: imp. · View Herald Transcript

araujo updated this object.Jan 18 2016, 6:35 AM

araujo edited edge metadata.

why would this even be optional?

In D4973#105377, @imp wrote:

why would this even be optional?

Very good question, I have replied on that PR, that it should not be even optional, however, with an embedded system some people might want it optional.

Best.

In D4973#105382, @araujo wrote:

In D4973#105377, @imp wrote:

why would this even be optional?

Very good question, I have replied on that PR, that it should not be even optional, however, with an embedded system some people might want it optional.

I've done embedded for a long time. I don't get this explanation at all. If it is optional, at all, it should be opt out. I'm having a hard time coming up with cases even in embedded where you wouldn't want this. Can you describe the actual use case? I'd imagine it would be exceedingly rare.

Best.

I do not see either the point of having this feature optional, if optional I find '-O' a weird option, I can't find an obvious translation for '-O', '-p' for protect seems straight forward :)

In D4973#105471, @imp wrote:

In D4973#105382, @araujo wrote:

In D4973#105377, @imp wrote:

why would this even be optional?

Very good question, I have replied on that PR, that it should not be even optional, however, with an embedded system some people might want it optional.

I've done embedded for a long time. I don't get this explanation at all. If it is optional, at all, it should be opt out. I'm having a hard time coming up with cases even in embedded where you wouldn't want this. Can you describe the actual use case? I'd imagine it would be exceedingly rare.

Best.

Me too, and a case that is not rare if you are not aware of it yet is: access point over a mountain far away from anywhere..I would prefer my syslogd be killed than my sshd and I can't acces my device anymore.

So yes, there are cases!

In D4973#105483, @bapt wrote:

I do not see either the point of having this feature optional, if optional I find '-O' a weird option, I can't find an obvious translation for '-O', '-p' for protect seems straight forward :)

Disagree! -p and -P are both used, take a look on syslogd(8). -O to protect against OOM as explained in the man page

bjk added a subscriber: bjk.Jan 18 2016, 6:40 PM

bjk added inline comments.

usr.sbin/syslogd/syslogd.8
31	Please update .Dd when a final change is committed.
257	I think this should be a capital O and not a zero?

In D4973#105483, @bapt wrote:

I do not see either the point of having this feature optional, if optional I find '-O' a weird option, I can't find an obvious translation for '-O', '-p' for protect seems straight forward :)

Disagree! -p and -P are both used, take a look on syslogd(8). -O to protect against OOM as explained in the man page

usr.sbin/syslogd/syslogd.8
257	You are right! It will be fixed soon.

Enable by default the protection against OOM killer.
Fix the typo in the manpage.
Bump DATE in the manpage.

Herald edited edge metadata. · View Herald TranscriptJan 22 2016, 2:38 AM

araujo added a reviewer: imp.Jan 22 2016, 2:41 AM

araujo removed a subscriber: imp.

Remove debug messages.

Sorry for the noisy.

Herald edited edge metadata. · View Herald TranscriptJan 22 2016, 2:43 AM

Now it is enabled by default with an opt out. Any comments?

I don't really like this patch at all.
This is a big behavioral change to syslogd.
Even though there is some special case because this is logging,
at some point you could ask, why not have this option in every daemon running on FreeBSD?

I'm not inclined to approve it.

In D4973#107844, @rodrigc wrote:

I don't really like this patch at all.
This is a big behavioral change to syslogd.
Even though there is some special case because this is logging,
at some point you could ask, why not have this option in every daemon running on FreeBSD?

I'm not inclined to approve it.

Yes, it could be an option create something that could set the protection in daemon of choice. I will think something about it.

On syslogd(8) I had the same problem with OOM killer couple years ago in a project, back on that time we made something pretty much similar like this patch.

I still think, this patch is valuable.

Best,

gshapiro added a subscriber: gshapiro.Jan 27 2016, 7:10 AM

gshapiro added inline comments.

usr.sbin/syslogd/syslogd.c
491	This should simply be "ProtectMode = 0;" -- no reason to continually decrement the value. Otherwise, if "-O" is specifed more than once, it actually reverses the intent since the later test (line 642) is simply testing if it is non-zero.