Re: [PATCH] oom_reaper: switch to struct list_head for reap queue

From: Aleksa Sarai
Date: Wed Feb 15 2017 - 04:01:22 EST

This is an extra pointer to task_struct and more lines of code to
accomplish the same thing. Why would we want to do that?

I don't think it's more "actual" lines of code (I think the wrapping is
inflating the line number count),

I too think it doesn't make sense to blow task_struct and the generated code.
And to me this patch doesn't make the source code more clean.

but switching it means that it's more in
line with other queues in the kernel (it took me a bit to figure out what
was going on with oom_reaper_list beforehand).

perhaps you can turn oom_reaper_list into llist_head then. This will also
allow to remove oom_reaper_lock. Not sure this makes sense too.

Actually, I just noticed that the original implementation is a stack not a queue. So the reaper will always reap the *most recent* task to get OOMed as opposed to the least recent. Since select_bad_process() will always pick worse processes first, this means that the reaper will reap "less bad" processes (lower oom score) before it reaps worse ones (higher oom score).

While it's not a /huge/ deal (N is going to be small in most OOM cases), is this something that we should consider?

RE: llist_head, the problem with that is that appending to the end is an O(n) operation. Though, as I said, n is not going to be very large in most cases.

Aleksa Sarai
Software Engineer (Containers)
SUSE Linux GmbH