←back to thread

144 points ksec | 1 comments | | HN request time: 0.508s | source
Show context
chasil ◴[] No.44466139[source]
So the assertion is that users with (critical) data loss bugs need complete solutions for recovery and damage containment with all possible speed, and without this "last mile" effort, stability will never be achieved.

The objection is the tiniest bug-fix windows get everything but the kitchen sink.

These are both uncomfortable positions to occupy, without doubt.

replies(2): >>44467021 #>>44468195 #
koverstreet ◴[] No.44467021[source]
No, the assertion is that the proper response to a bug often (and if it's high impact - always) involves a lot more than just the bugfix.

And the whole reason for a filesystem's existence is to store and maintain your data, so if that is what the patch if for, yes, it should be under consideration as a hotfix.

There's also the broader context: it's a major problem for stabilization if we can't properly support the people using it so they can keep testing.

More context: the kernel as a whole is based on fixed time tables and code review, which it needs because QA (especially automated testing) is extremely spotty. bcachefs's QA, both automated testing and community testing, is extremely good, and we've had bugfix patchsets either held up or turn into flamewars because of this mismatch entirely too many times.

replies(4): >>44467217 #>>44467479 #>>44468100 #>>44470493 #
WesolyKubeczek ◴[] No.44467479[source]
> No, the assertion is that the proper response to a bug often (and if it's high impact - always) involves a lot more than just the bugfix.

Then what you do is you try to split your work in two. You could think of a stopgap measure or a workaround which is small, can be reviewed easily, and will reduce the impact of the bug while not being a "proper" fix, and prepare the "properer" fix when the merge window opens.

I would ask, since the bug probably lived since the last stable release, how come it fell through the crack and had only been noticed recently? Could it be that not all setups are affected? If so, can't they live with it until the next merge window?

By making a "feature that fixes the bug for real", you greatly expand the area in which new, unknown bugs may land, with very little time to give it proper testing. This is inevitable, evident by the simple fact that the bug you were trying to fix exists. You can be good, but not that good. Nobody is that good. If anybody was that good, they wouldn't have the bug in the first place.

If you have commercial clients who use your filesystem and you have contractual obligations to fix their bugs and keep their data intact, you could (I'd even say "should") maintain an out-of-tree version with its own release and bugfix schedule. This is IMO the only reasonable way to have it, because the kernel is a huge administrative machine with lots of people, and by mainlining stuff, you necessarily become co-dependent on the release schedule for the whole kernel. I think a conflict between kernel's release schedule and contractual obligations, if you have any, is only a matter of time.

replies(1): >>44468619 #
koverstreet ◴[] No.44468619[source]
> Then what you do is you try to split your work in two. You could think of a stopgap measure or a workaround which is small, can be reviewed easily, and will reduce the impact of the bug while not being a "proper" fix, and prepare the "properer" fix when the merge window opens.

That is indeed what I normally do. For example, 6.14 and 6.15 had people discovering btree iterator locking bugs (manifesting as assertion pops) while running evacuates on large filesystems (it's hard to test a sufficiently deep tree depth in virtual machine tests with our large btree nodes); some small hotfixes went out in rc kernels, but the majority of the work (a whole project to add assertions for path->should_be_locked, which should shut these down for good) waited until the 6.16 merge window.

That was for a less critical bug - your machine crashing is somewhat less severe than losing a filesystem.

In this case, we had a bug pop up in 6.15 where the link count in the VFS inode getting screwed up caused an inode to be deleted that shouldn't have been - a subvolume root - and then an untested repair path took out the entire subvolume.

Ouuuuch.

That's why the repair code was rushed; it had already gotten one filesystem back, and I'd just gotten another report of someone else hitting it - and for every bug report there are almost always more people who hit it and don't report it.

And considering that a lot of people running bcachefs now are getting it from distro kernels and don't know how to build kernels - that is why it was important to get this out quickly through the normal channels.

In addition, the patch wasn't risky, contrary to what Ted was saying. It's a code path that's very well covered by automated tests, including KASAN/UBSAN/lockdep variants - those would exploded if this patch was incorrect.

When to ship a patch is always a judgement call, and part of how you make that call is how well your QA process can guarantee the patch is correct. Part of what was going on here is a disconnect between those of us who do make heavy use of modern QA infrastructure and those who do it the old school way, relying heavily on manual review and long testing periods for rc kernels.

replies(1): >>44475096 #
1. WesolyKubeczek ◴[] No.44475096[source]
> In this case, we had a bug pop up in 6.15 where the link count in the VFS inode getting screwed up caused an inode to be deleted that shouldn't have been - a subvolume root - and then an untested repair path took out the entire subvolume.

I would rather make sure this path was never hit for rc, to minimize the damage. The fact alone that it didn’t pop up until late in the 6.15 cycle could hint at some specific circumstances the bug manifested on, and those could be described and avoided.

And I think there could be a mediocre way to get by until the next merge window in which a superior solution could be presented.

I don’t want to sound as if I’m an expert in how to do VFS, because I’m not. I’m, however, an expert in how to be “correcter than others” which has cost me getting kicked out of jobs before. I hope I have learned better since, and at the time I have been very, very stubborn (they wouldn’t have kicked me out otherwise).

There is this bit when working with others that you will likely go with solutions you deem more mediocre than the theoretically best solution, or that experienced people will say “no” to your ideas or solutions and you accept it instead of seeking quarrel in spite of them obviously not understanding you (spoiler: this is not so). But if you show that you’re willing to work with other as a single unit, you will be listened to, appreciated, and concessions will be made for you too. This is not necessarily about the kernel, it’s about working in a team in general.

I don’t have a dog in this fight, but I’ve been “that guy” before and I regret it took me that long to realize this and mend my ways. Hope it doesn’t keep happening to you.