Re: [GIT PULL] block-bio_iov_iter_export

Re: [GIT PULL] block-bio_iov_iter_export

[Christoph Hellwig]

Umm,

besides adding exports without in-tree users, this is a patch that's
never seen any relevant mailing list, in a pull request that the
maintainer hasn't seen.  That's now exactly how Linux development works,
does it?

Re: [GIT PULL] block-bio_iov_iter_export

[Kent Overstreet]

On Thu, Oct 16, 2025 at 11:13:46PM -0700, Christoph Hellwig wrote:
> Umm,
> 
> besides adding exports without in-tree users, this is a patch that's
> never seen any relevant mailing list, in a pull request that the
> maintainer hasn't seen.  That's now exactly how Linux development works,
> does it?

Christoph, I wrote that code /for bcachefs/; the rest of you decided it
was nice and started using it too.

Then you removed the export talking about the "abuse" of bcachefs using
it. WTF?

Re: [GIT PULL] block-bio_iov_iter_export

[Christoph Hellwig]

On Fri, Oct 17, 2025 at 09:31:59AM -0400, Kent Overstreet wrote:
> On Thu, Oct 16, 2025 at 11:13:46PM -0700, Christoph Hellwig wrote:
> > Umm,
> > 
> > besides adding exports without in-tree users, this is a patch that's
> > never seen any relevant mailing list, in a pull request that the
> > maintainer hasn't seen.  That's now exactly how Linux development works,
> > does it?
> 
> Christoph, I wrote that code /for bcachefs/; the rest of you decided it
> was nice and started using it too.

In fact this version was written by me, giving you the attribution
because I stole a cool idea from you.  But none of this actually
matters, there's not magic exception just because someone wrote the
code.

> Then you removed the export talking about the "abuse" of bcachefs using
> it. WTF?

The random NULL bdev check that breaks the proper splitting.  We told
told you that's not the way to go, but you just sent it directly to Linus
instead of reworking it.  And this then got into the way of the rework
Keith did to support arbitrarily small memory alignments.  Fortunately
we could clean this up properly now.

Re: [GIT PULL] block-bio_iov_iter_export

[Kent Overstreet]

On Mon, Oct 20, 2025 at 02:35:40AM -0700, Christoph Hellwig wrote:
> On Fri, Oct 17, 2025 at 09:31:59AM -0400, Kent Overstreet wrote:
> > On Thu, Oct 16, 2025 at 11:13:46PM -0700, Christoph Hellwig wrote:
> > > Umm,
> > > 
> > > besides adding exports without in-tree users, this is a patch that's
> > > never seen any relevant mailing list, in a pull request that the
> > > maintainer hasn't seen.  That's now exactly how Linux development works,
> > > does it?
> > 
> > Christoph, I wrote that code /for bcachefs/; the rest of you decided it
> > was nice and started using it too.
> 
> In fact this version was written by me, giving you the attribution
> because I stole a cool idea from you.  But none of this actually
> matters, there's not magic exception just because someone wrote the
> code.

The implementation has morphed given multipage bvecs and iov_iters, but
otherwise it looks structurally much the same as the version I
originally introduced.

Please attribute correctly, and that would've included CCing me on the
patch that dropped the EXPORT_SYMBOL().

> > Then you removed the export talking about the "abuse" of bcachefs using
> > it. WTF?
> 
> The random NULL bdev check that breaks the proper splitting.  We told
> told you that's not the way to go, but you just sent it directly to Linus
> instead of reworking it.  And this then got into the way of the rework
> Keith did to support arbitrarily small memory alignments.  Fortunately
> we could clean this up properly now.

So, you "told" the person that originally authored not only this code
but all our modern bio splitting infastructure that this isn't the way
to go?

That's quite the way of framing it.

The way you're doing it with bdev_logical_block_size() is just wrong -
even for single device filesystems! - because it's the filesystem
blocksize that's relevant here and that isn't necessarily going to match
(even if it matched when the filesystem was formatted, filesystems can
be moved to different block devices).

You'll figure this out sooner or later as bs > ps becomes more
prevalent...

Re: [GIT PULL] block-bio_iov_iter_export

[Christoph Hellwig]

On Mon, Oct 20, 2025 at 08:56:59AM -0400, Kent Overstreet wrote:
> The implementation has morphed given multipage bvecs and iov_iters, but
> otherwise it looks structurally much the same as the version I
> originally introduced.

Not a pissing context, but I introduced it.  I attributed the git
authorship you because it fundamentally it based on your idea but with a
lot of tweaks.  I and many others do this to give proper credit.

> Please attribute correctly, and that would've included CCing me on the
> patch that dropped the EXPORT_SYMBOL().

No, we don't Cc the author of each line of code or even function.  The
relevant maintainer here is Jens.

> The way you're doing it with bdev_logical_block_size() is just wrong -
> even for single device filesystems! - because it's the filesystem
> blocksize that's relevant here and that isn't necessarily going to match
> (even if it matched when the filesystem was formatted, filesystems can
> be moved to different block devices).

I'm not sure what you are talking about, but the changes you seem to
be complaining about are making the alignment boundary a caller provided
argument.  Which seems to be what you're arguing for here?  Either way
this is the wrong venue.  If you want to change something sent patches
following the usual guidelines to the maintainer.

Re: [GIT PULL] block-bio_iov_iter_export

[Kent Overstreet]

On Mon, Oct 20, 2025 at 06:21:36AM -0700, Christoph Hellwig wrote:
> On Mon, Oct 20, 2025 at 08:56:59AM -0400, Kent Overstreet wrote:
> > The implementation has morphed given multipage bvecs and iov_iters, but
> > otherwise it looks structurally much the same as the version I
> > originally introduced.
> 
> Not a pissing context, but I introduced it.  I attributed the git
> authorship you because it fundamentally it based on your idea but with a
> lot of tweaks.  I and many others do this to give proper credit.

Christoph, I don't know what you're claiming here. I see no tweaks in
the original patch, that's all code I wrote.

> > Please attribute correctly, and that would've included CCing me on the
> > patch that dropped the EXPORT_SYMBOL().
> 
> No, we don't Cc the author of each line of code or even function.  The
> relevant maintainer here is Jens.

I always try to CC people with relevant expertise, and that _definitely_
includes the person who authored the code being changed.

And considering the multiple times I've had to track down bugs you and
Jens introduced into code I wrote without CCing me...

> > The way you're doing it with bdev_logical_block_size() is just wrong -
> > even for single device filesystems! - because it's the filesystem
> > blocksize that's relevant here and that isn't necessarily going to match
> > (even if it matched when the filesystem was formatted, filesystems can
> > be moved to different block devices).
> 
> I'm not sure what you are talking about, but the changes you seem to
> be complaining about are making the alignment boundary a caller provided
> argument.  Which seems to be what you're arguing for here?

You brought up the bdev NULL check.

> Either way this is the wrong venue.  If you want to change something
> sent patches following the usual guidelines to the maintainer.

There was no need for you to drop the EXPORT_SYMBOL.

What I want to know is, is this going to become a pattern?

I can vendorize this one function, but If you're going to make a habit
of ripping out exports and functionality bcachefs depends on, I can't
expect I'll always be able to. There's open bug on the bcachefs bug
tracker for 6.18 support, and people are just patching the kernel to
deal with this. I'll just pass the bcachefs enablement patches to the
distros if it looks like that's going to be less of a hassle.

With the lib/Kconfig patch to make library code user selectable that you
also nacked, perhaps that's the safer route here.

Re: [GIT PULL] block-bio_iov_iter_export

[Christoph Hellwig]

On Mon, Oct 20, 2025 at 10:49:57AM -0400, Kent Overstreet wrote:
> Christoph, I don't know what you're claiming here. I see no tweaks in
> the original patch, that's all code I wrote.

Then look closer.

> There was no need for you to drop the EXPORT_SYMBOL.
> What I want to know is, is this going to become a pattern?

Of course there was.  The kernel doesn't keep unused code around,
including symbols.  So anything that is unused will eventually be
garbage collected.  There's even folks around that run scripts and
automate it (David Alan Gilbert is the most active one currently).

> I can vendorize this one function, but If you're going to make a habit
> of ripping out exports and functionality bcachefs depends on, I can't
> expect I'll always be able to.

I'm not sure why you're turning this personal and singling me out.
Yes, unused exports and code are removed all the time, and that's a
feature and not a bug.

Re: [GIT PULL] block-bio_iov_iter_export

[Kent Overstreet]

On Mon, Oct 20, 2025 at 07:54:03AM -0700, Christoph Hellwig wrote:
> On Mon, Oct 20, 2025 at 10:49:57AM -0400, Kent Overstreet wrote:
> > Christoph, I don't know what you're claiming here. I see no tweaks in
> > the original patch, that's all code I wrote.
> 
> Then look closer.
> 
> > There was no need for you to drop the EXPORT_SYMBOL.
> > What I want to know is, is this going to become a pattern?
> 
> Of course there was.  The kernel doesn't keep unused code around,
> including symbols.  So anything that is unused will eventually be
> garbage collected.  There's even folks around that run scripts and
> automate it (David Alan Gilbert is the most active one currently).

A comment would be an easy solution to that.

> 
> > I can vendorize this one function, but If you're going to make a habit
> > of ripping out exports and functionality bcachefs depends on, I can't
> > expect I'll always be able to.
> 
> I'm not sure why you're turning this personal and singling me out.
> Yes, unused exports and code are removed all the time, and that's a
> feature and not a bug.

If this is a serious question, READ FUA is another answer :)

Re: [GIT PULL] block-bio_iov_iter_export

[Greg Kroah-Hartman]

On Mon, Oct 20, 2025 at 10:49:57AM -0400, Kent Overstreet wrote:
> There was no need for you to drop the EXPORT_SYMBOL.

We don't keep symbols exported when there is no in-kernel user of that
export, sorry.  Otherwise, that way lies madness.

thanks,

greg k-h