linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [GIT PULL] block-bio_iov_iter_export
       [not found] <ov54jszhism7mbeu74vtyoysxnx3y3tsjbj5esszlrx3edq77s@j2vtyy45gsna>
@ 2025-10-17  6:13 ` Christoph Hellwig
  2025-10-17 13:31   ` Kent Overstreet
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-10-17  6:13 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: Linus Torvalds, linux-kernel, Jens Axboe, Greg Kroah-Hartman,
	linux-block

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?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] block-bio_iov_iter_export
  2025-10-17  6:13 ` [GIT PULL] block-bio_iov_iter_export Christoph Hellwig
@ 2025-10-17 13:31   ` Kent Overstreet
  2025-10-20  9:35     ` Christoph Hellwig
  0 siblings, 1 reply; 9+ messages in thread
From: Kent Overstreet @ 2025-10-17 13:31 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linus Torvalds, linux-kernel, Jens Axboe, Greg Kroah-Hartman,
	linux-block

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?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] block-bio_iov_iter_export
  2025-10-17 13:31   ` Kent Overstreet
@ 2025-10-20  9:35     ` Christoph Hellwig
  2025-10-20 12:56       ` Kent Overstreet
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-10-20  9:35 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: Christoph Hellwig, Linus Torvalds, linux-kernel, Jens Axboe,
	Greg Kroah-Hartman, linux-block

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] block-bio_iov_iter_export
  2025-10-20  9:35     ` Christoph Hellwig
@ 2025-10-20 12:56       ` Kent Overstreet
  2025-10-20 13:21         ` Christoph Hellwig
  0 siblings, 1 reply; 9+ messages in thread
From: Kent Overstreet @ 2025-10-20 12:56 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linus Torvalds, linux-kernel, Jens Axboe, Greg Kroah-Hartman,
	linux-block

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] block-bio_iov_iter_export
  2025-10-20 12:56       ` Kent Overstreet
@ 2025-10-20 13:21         ` Christoph Hellwig
  2025-10-20 14:49           ` Kent Overstreet
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-10-20 13:21 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: Christoph Hellwig, Linus Torvalds, linux-kernel, Jens Axboe,
	Greg Kroah-Hartman, linux-block

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.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] block-bio_iov_iter_export
  2025-10-20 13:21         ` Christoph Hellwig
@ 2025-10-20 14:49           ` Kent Overstreet
  2025-10-20 14:54             ` Christoph Hellwig
  2025-10-20 15:13             ` Greg Kroah-Hartman
  0 siblings, 2 replies; 9+ messages in thread
From: Kent Overstreet @ 2025-10-20 14:49 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linus Torvalds, linux-kernel, Jens Axboe, Greg Kroah-Hartman,
	linux-block

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] block-bio_iov_iter_export
  2025-10-20 14:49           ` Kent Overstreet
@ 2025-10-20 14:54             ` Christoph Hellwig
  2025-10-20 16:42               ` Kent Overstreet
  2025-10-20 15:13             ` Greg Kroah-Hartman
  1 sibling, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-10-20 14:54 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: Christoph Hellwig, Linus Torvalds, linux-kernel, Jens Axboe,
	Greg Kroah-Hartman, linux-block

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.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] block-bio_iov_iter_export
  2025-10-20 14:49           ` Kent Overstreet
  2025-10-20 14:54             ` Christoph Hellwig
@ 2025-10-20 15:13             ` Greg Kroah-Hartman
  1 sibling, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2025-10-20 15:13 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: Christoph Hellwig, Linus Torvalds, linux-kernel, Jens Axboe,
	linux-block

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL] block-bio_iov_iter_export
  2025-10-20 14:54             ` Christoph Hellwig
@ 2025-10-20 16:42               ` Kent Overstreet
  0 siblings, 0 replies; 9+ messages in thread
From: Kent Overstreet @ 2025-10-20 16:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linus Torvalds, linux-kernel, Jens Axboe, Greg Kroah-Hartman,
	linux-block

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 :)

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-10-20 16:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <ov54jszhism7mbeu74vtyoysxnx3y3tsjbj5esszlrx3edq77s@j2vtyy45gsna>
2025-10-17  6:13 ` [GIT PULL] block-bio_iov_iter_export Christoph Hellwig
2025-10-17 13:31   ` Kent Overstreet
2025-10-20  9:35     ` Christoph Hellwig
2025-10-20 12:56       ` Kent Overstreet
2025-10-20 13:21         ` Christoph Hellwig
2025-10-20 14:49           ` Kent Overstreet
2025-10-20 14:54             ` Christoph Hellwig
2025-10-20 16:42               ` Kent Overstreet
2025-10-20 15:13             ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).