LWN: Comments on "Stabilizing per-VMA locking"

Slowing down fork-heavy workloads in favour of thread-heavy workloads by THIS MUCH?

mirabilos — Fri, 21 Jul 2023 15:09:35 +0000

An immediate idea to mitigate this: keep a process-global mmap lock until the first thread is created, switch to per-VMA locking only then.

Is that workable?

Many many standard Unix tools don’t use any threads at all, while the standard workflow uses loops and pipes and is therefore fork-heavy.

Stabilizing per-VMA locking

jeremyhetzler — Fri, 21 Jul 2023 12:06:46 +0000

Was this test-case, that triggered the bug, added to the test suite? That would be the best way to prevent similar bugs in the future.

Stabilizing per-VMA locking

khim — Thu, 20 Jul 2023 14:02:21 +0000

>But for that to work you need to understand your ownership model clearly, otherwise you can't encode it in the type system.

And if you couldn't do that then your program can not be compiled and thus wouldn't have any bugs.

The question is not whether Rust may prevent such bugs. It absolutely could do that, it's designed for that and it works well.

The question is whether price is too high and if code which would satisfy Rust compiler would also satisfy performance demands of kernel developers, too.

That is still an open question, that's true.

2.4.10

Paf — Sun, 16 Jul 2023 19:30:12 +0000

Yeah, I can’t argue with the results. That stood out but I was in fact talking as much about the whole process - the differing branches and the seeming lack of maintainers except Linus. It feels a little like watching a kind of multi-mode high traffic near-chaos flow, the kind that seems - to someone used to orderly traffic - like it should lead to constant crashes but somehow doesn’t (usually :x).

Stabilizing per-VMA locking

ringerc — Sat, 15 Jul 2023 10:37:39 +0000

Ah, that explains it, I missed that it had to be MT. Postgres would never hit that every in unusual deployments like those using PL/Java.

2.4.10

corbet — Sat, 15 Jul 2023 00:40:51 +0000

The VM switch in 2.4 seemed insane at the time as well. It did seem to stabilize a lot of the persistent problems that had been plaguing that kernel, though; it's been a long time since I've heard anybody who thinks it was the wrong decision.

Stabilizing per-VMA locking

Paf — Sat, 15 Jul 2023 00:25:39 +0000

From the perspective of someone who wasn’t around then and has only seen basically the current process, that all sounds just *insane* as a dev process.

Stabilizing per-VMA locking

willy — Fri, 14 Jul 2023 19:40:38 +0000

The task calling fork() must be multithreaded to encounter this bug. Most multithreaded tasks don't call fork(). Most tasks that call fork() (including pgbench) are not multithreaded. It's not too surprising that it took months to be uncovered (that is, months since Suren started posting this patch series, not months since it was released in an official kernel).

Stabilizing per-VMA locking

mb — Fri, 14 Jul 2023 14:05:08 +0000

>But for that to work you need to understand your ownership model clearly,
>otherwise you can't encode it in the type system.

If you don't understand the ownership model, you can't write correct C code either. The resulting code may be correct by luck, at best.

>I think in this case here it was more an issue of not clearly understanding the
>ownership model of vma structs

Exactly.
Because it was not enforced and not even encouraged by the language to think about it.

>not so much a missed case that the compiler could have spotted for you.

I'm not talking about that kind of thing.
I'm talking about Rust encouraging you to think about ownership. This is many levels above a compiler run.
Programming Rust puts your brain into a completely different mode of thinking. And that's what makes the real difference.

Stabilizing per-VMA locking

sam_c — Fri, 14 Jul 2023 12:30:46 +0000

I took the above comment as being about the fork problem in 6.4 rather than the stack rot issue from 6.1.

Stabilizing per-VMA locking

farnz — Fri, 14 Jul 2023 10:25:41 +0000

It's complicated. The newest kernel is, by definition, the best kernel the developers know how to produce, but that does not necessarily make it the best for your workload, because it may have new bugs that matter to you, while the old bugs don't matter to you.

And one component of "please use the latest" is that bug reports become less valuable the longer the gap between bug introduced, and bug found; if you report a bug that's not present in 6.4, but is present in 6.5-rc1, not only are there fewer commits to consider that could possibly have exposed the bug, but the people who will fix your bug have the context around why something works the way it does in recent memory. If you report a bug as added in 5.12-rc1 and still present in 6.5-rc1, but not in 5.11, you're still keeping the commit count down, but now you're asking the developers to remember why changes were made in the 5.12 time frame, and what the intended effect was.

Stabilizing per-VMA locking

sima — Fri, 14 Jul 2023 07:06:09 +0000

I'm not sure rust would have helped here a lot. Rust is really good at enforcing ownership models and pretty flexible at that. But for that to work you need to understand your ownership model clearly, otherwise you can't encode it in the type system. And you need to encode it all yourself for custom data structure protection schemes like this one.

I think in this case here it was more an issue of not clearly understanding the ownership model of vma structs, and not so much a missed case that the compiler could have spotted for you. Where rust might help is that it allows you to roll out the ownership model first (in the type system) without changing the locking, and then once you're fairly confident that you got it right, you flip the switch (and watch things blow up if you missed something).

Stabilizing per-VMA locking

mb — Fri, 14 Jul 2023 06:26:50 +0000

> Whether, say, a VMA abstraction written in Rust could truly ensure that accesses use proper
> locking while maintaining performance has not yet been proven in the kernel context, though.

Well, I think that such performance critical paths would either use safe code or they would be marked unsafe {} and use some lockless performance tricks.
The unsafe-block by itself would probably help to raise some eyebrows when doing such fundamental locking changes.

Stabilizing per-VMA locking

alison — Fri, 14 Jul 2023 04:54:26 +0000

Thanks for posting that link into the Wayback Machine: it made me laugh.

Stabilizing per-VMA locking

andresfreund — Fri, 14 Jul 2023 04:02:47 +0000

I actually happened to run a bunch of postgres benchmarks on a vulnerable kernel, without, as far as I know it, encountering the issue. I suspect it's because postmaster just doesn't have a lot of page faults.

Stabilizing per-VMA locking

jlbec — Fri, 14 Jul 2023 02:16:02 +0000

Significant VM changes in major releases that take time and consternation to sort out are time-honored tradition.

Stabilizing per-VMA locking

ringerc — Thu, 13 Jul 2023 22:34:25 +0000

Wow. This bug would've made postgres extremely sad, and should've turned up reasonably quickly in a pgbench workload.

Interesting that it got so far, given that postgres has been a useful tool to exercise other MM kernel issues in the past.

Stabilizing per-VMA locking

Paf — Thu, 13 Jul 2023 22:23:22 +0000

“ Kernel developers often make the point that the newest kernels are the best ones that the community knows how to make and that users should not hesitate to upgrade to them. Responding more quickly when an upgrade turns out to be a bad idea would help to build confidence in that advice.”

It seems so obvious that the truth of “newest is best” is more complicated than this (I’m not saying those involved don’t know this, just it helps me to say it directly). The basic logic of it is obviously correct, but at the same time, it’s common that new things have new problems not yet found through greater exposure. I don’t really have an answer, maybe it’s to hugely increase testing of kernels before they come out. But it seems likely there will still be a sweet spot with stable kernels that are just a bit older base with more fixes, whatever we may say about the best we know how to produce. :/

Stabilizing per-VMA locking

dskoll — Thu, 13 Jul 2023 21:02:40 +0000

Anecdote: Firefox began crashing occasionally (1-2x per day) when I switched to kernel 6.4. Since I upgraded to 6.4.3 two days ago, it has not crashed. I don't know for sure it was the kernel, but it seems likely.