Re: [PATCH v2 1/8] lib/printbuf: New data structure for heap-allocated strings

From: Kent Overstreet
Date: Fri Apr 22 2022 - 02:06:56 EST


On Fri, Apr 22, 2022 at 07:52:14AM +0200, Christoph Hellwig wrote:
> On Fri, Apr 22, 2022 at 01:40:15AM -0400, Kent Overstreet wrote:
> > Wasn't just bcachefs, it affected bcache too, as Coly also reported.
>
> Well, I've not seen a good bug report for that, but I'd gladly look at it.

Thanks. It's been awhile but I'll see if I can dig up the original bug report
tomorrow.

> > And I wrote
> > that code originally (and the whole fucking modern bvec iter infrastracture,
> > mind you) so please don't lecture me on making assumptions on block layer
> > helpers.
>
> Well, most of what we have is really from Ming. Because your original
> idea was awesome, but the code didn't really fit. Then again I'm not
> sure why this even matters.

Didn't fit how? And Ming extended it to multipage bvecs based on my proposal.

> I'm also relly not sure why you are getting so personal.

Put yourself my shoes, I've honestly found you to be hardheaded and exceedingly
difficult to work with for a very long time.

But I'll bite my tongue for now, because if you'll start listening to bug
reports that will go a long way towards easing things, and we've got LSF coming
up so maybe we can hash things out over beers.

> > Now yes, I _could_ do a wholesale conversion of seq_buf to printbuf and delete
> > that code, but doing that job right, to be confident that I'm not introducing
> > bugs, is going to take more time than I really want to invest right now. I
> > really don't like to play fast and loose with that stuff.
>
> Even of that I'd rather see a very good reason first. seq_bufs have been
> in the kernel for a while and seem to work fine. If you think there are
> shortcomings please try to improve it, not replace or duplicate it.
> Sometimes there might be a good reason to replace exiting code, but it
> rather have to be a very good reason.

When the new version is semantically different from the old version it makes it
a lot easier to deal with the merge conflicts later when forward/backporting
stuff by giving the new version a new name.

Anyways, I'll have a chat with Steven Rostedt about it since I believe he wrote
the original code.