Linus Torvalds Isn't Happy With Some Of The Bcachefs Code For Linux 6.9
Since the Bcachefs file-system was upstreamed in the Linux 6.7 kernel it's been humming along fairy well. But today the Bcachefs feature updates were sent in for the Linux 6.9 merge window and Linus Torvalds isn't happy about some of the proposed code.
The Bcachefs code submitted for Linux 6.9 includes some prep work toward a user-space interface for walking subvolumes, improvements to directory structure checking, improved journal pipelining for better performance, discard path improvements that are more efficient, and other optimizations. The pull request of Bcachefs changes for Linux 6.9 is summed up by maintainer Kent Overstreet as:
But the code rubbing Linus Torvalds the wrong way are the patches moving some elements of the Bcachefs code out to some library-type code so that it can be easily re-used by other file-systems -- XFS was the expressed file-system interested in potentially re-using some Bcachefs functions.
Linus Torvalds responded to that Bcachefs pull request with:
After Overstreet argued his case, Torvalds added:
So as it stands now, Linus Torvalds isn't accepting this Bcachefs pull request for the Linux 6.9 kernel due to the proposed generic library code. We'll see if a new pull request comes in the days ahead with those patches dropped or otherwise reworked to satisfy the Linux creator.
Update: A revised pull request was merged for Linux 6.9 without the library code.
The Bcachefs code submitted for Linux 6.9 includes some prep work toward a user-space interface for walking subvolumes, improvements to directory structure checking, improved journal pipelining for better performance, discard path improvements that are more efficient, and other optimizations. The pull request of Bcachefs changes for Linux 6.9 is summed up by maintainer Kent Overstreet as:
bcachefs updates for 6.9
- Subvolume children btree; this is needed for providing a userspace interface for walking subvolumes, which will come later
- Lots of improvements to directory structure checking
- Improved journal pipelining, significantly improving performance on high iodepth write workloads
- Discard path improvements: the discard path is more efficient, and no longer flushes the journal unnecessarily
- Buffered write path can now avoid taking the inode lock
- Pull out various library code for use in XFS: time stats, mean_and_variance, darray, eytzinger, thread_with_file
- new mm helper: memalloc_flags_{save|restore}
- mempool now does kvmalloc mempools
But the code rubbing Linus Torvalds the wrong way are the patches moving some elements of the Bcachefs code out to some library-type code so that it can be easily re-used by other file-systems -- XFS was the expressed file-system interested in potentially re-using some Bcachefs functions.
Linus Torvalds responded to that Bcachefs pull request with:
The "make random bcachefs code be a library function" stuff I looked at, decided is senseless, and ended up meaning that I'm not pulling this without a lot more explanation (and honestly, I don't think the explanations would hold water).
That "stdio_redirect_printf()" and darray_char stuff is just horrendous interfaces with no explanations. The interfaces are disgusting.
Keep it in your own code where it belongs, don't try to make it some generic library thing.
And if you *do* make it a library thing, it needs to be
(a) much more explained
(b) have much saner naming, and fewer disgusting and completely nonsensical interfaces ("DARRAY()").
And no, finding one other filesystem to share this kind of code is not sufficient to try to claim it's a sane interface and sane naming.
But the main dealbreaker is the insane math.
And dammit, we talked about the idiotic "mean and variance" garbage long ago. It was wrong back then, it's *still* wrong.
You didn't explain why it couldn't use the *much* simpler MAD (median absolute deviation) instead of using variance.
That bad decision directly results in that pointless use of overly complex 128-bit math.
I called it insanely over-engineered back then, and as far as I can tell, absolutely *NOTHING* has changed apart from some slight type name details.
As long as you made it some kind of bcachefs-only thing, I don't mind.
But now you're trying to push this garbage as some kind of generic library code that others would use, and that immediately means that I *do* mind insanely overengineered interfaces.
The time_stats stuff otherwise looks at leask like a sane interface with names and uses, but the use of that horrendous infrastructure scuttles it.
After Overstreet argued his case, Torvalds added:
The code for the weighted version literally doesn't change.
The variance value is different, but the difference between MAD and standard deviation is basically just a constant factor (which will be different for different distributions, but so what? Any _particular_ case will have a particular distribution).
So why would a constant factor make _any_ difference for any exponential weighting?
Anyway, feel free to keep your code in bcachefs.
And maybe xfs even wants to copy that code. I don't care, it seems stupid, but that's a filesystem choice.
But if we're making it a generic kernel library, it needs to be sane. Not making people do 64-bit square roots and 128-bit divides just for a random statistical element.
So as it stands now, Linus Torvalds isn't accepting this Bcachefs pull request for the Linux 6.9 kernel due to the proposed generic library code. We'll see if a new pull request comes in the days ahead with those patches dropped or otherwise reworked to satisfy the Linux creator.
Update: A revised pull request was merged for Linux 6.9 without the library code.
58 Comments