|
|
Subscribe / Log in / New account

Fighting passive aggression should not entirely fall on the victim

Fighting passive aggression should not entirely fall on the victim

Posted Feb 7, 2025 17:50 UTC (Fri) by koverstreet (✭ supporter ✭, #4296)
In reply to: Fighting passive aggression should not entirely fall on the victim by Wol
Parent article: The selfish contributor revisited

So you're processing a digraph and he hadn't learned anything past linked lists? Hah...


to post comments

Fighting passive aggression should not entirely fall on the victim

Posted Feb 7, 2025 21:14 UTC (Fri) by Wol (subscriber, #4433) [Link] (1 responses)

The killer though was he ignored the comment spelling out exactly why that code was needed! And then wondered why the predicted consequences happened!

Cheers,
Wol

Fighting passive aggression should not entirely fall on the victim

Posted Feb 7, 2025 21:46 UTC (Fri) by koverstreet (✭ supporter ✭, #4296) [Link]

I'll one up you.

So before I started working on the kernel, splitting bios (block layer IOs) was an unholy mess. There was no reasonable way to do it if the bio was more than a single segment, so upper layers would query lower layers "can I add another page to this bio?" as they were building it up. Oof...

This was all stuff that predated the more modern stacking block drivers (dm, bcache); it kind of worked for md where the restrictions were static, but quickly fell over if they were dynamic. And it was a whole pile of fragile, brittle slow code.

So I got rid of that and made efficient bio splitting possible by introducing a novel concept - the iterator. That made the biovec immutable, meaning it could be shared by splits. Novel stuff. It also meant that you could take a bio you were processing (in the middle of the io stack), stash a copy of the iterator somewhere, submit it, and then on completion restore the iterator to its previous state and walk precisely the same segments you saw when it was originally submitted to you (say for bouncing, decryption, what have you).

Deleted a ton of code from the block layer and device mapper, and bcache was pretty heavily reliant on all this stuff. Worked great, but..

It seems the maintainers never got the memo on things you can and cannot do with bio iterators. Saving and restoring iterators works great, because then it doesn't matter what the lower layer does with the bio (and it can advance the iterator, truncate it, or both, or neither). But an iterator was too big for them, they wanted to be able to restore the bio to its previous state by just rewinding it (by some number of bytes they pulled out of the ether).

So they merged an entire standard normal looking API to do this completely broken "bio_rewind()" thing, and then drivers started using it in predictably broken ways. And I didn't find out about it by getting CC'd on any patches - no, I found out about it through reports of bcache corrupting data; much shouting was required to get it reverted. And then they tried to merge it again...


Copyright © 2025, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds