3 ms·

> 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 r

> 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.

WesolyKubeczek 1y ago
> 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.

WesolyKubeczek 1y ago

> 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.