Re: [GIT PULL] bcachefs

[Brian Foster]

On Thu, Sep 07, 2023 at 04:39:46PM -0400, Kent Overstreet wrote:
> On Wed, Sep 06, 2023 at 05:03:06PM -0700, Kees Cook wrote:
> > On Wed, Sep 06, 2023 at 03:28:47PM -0700, Nathan Chancellor wrote:
> > > Hi Kent,
> > > 
> > > On Sat, Sep 02, 2023 at 11:25:55PM -0400, Kent Overstreet wrote:
> > > > here's the bcachefs pull request, for 6.6. Hopefully everything
> > > > outstanding from the previous PR thread has been resolved; the block
> > > > layer prereqs are in now via Jens's tree and the dcache helper has a
> > > > reviewed-by from Christain.
> > > 
> > > I pulled this into mainline locally and did an LLVM build, which found
> > > an immediate issue. It appears the bcachefs codes uses zero length
> > 
> > It looks like this series hasn't been in -next at all? That seems like a
> > pretty important step.
> > 
> > Also, when I look at the PR, it seems to be a branch history going
> > back _years_. For this kind of a feature, I'd expect a short series of
> > "here's the code" in incremental additions (e.g. look at the x86 shstk
> > series), not the development history from it being out of tree -- this
> > could easily lead to ugly bisection problems, etc.
> 
> Chris already commented on this, but - we really want the full history
> available, that's important context for anyone working on the code going
> forward. Brian was just mentioning digging through the history for
> context in the meeting last Tuesday, and I've been going to a lot of
> effort to make sure every commit builds and runs.
> 

Yeah.. IMO the main advantages of a sort of squashed down/sanitized git
history is to either aid in code review or just clean up a history that
is aesthetically a mess. For the former, the consensus seems to be that
no one person is going to sit down and review the entire codebase, but
rather folks have been digging into peripheral areas they have
experience in (i.e., locking, pagecache, etc.) to call out any major
concerns. I believe Kent has also offered to give pointers or just sit
down with anybody who needs assistance to navigate the codebase for
review purposes. For the latter, ISTM that bcachefs has pretty much
followed kernel patch conventions, with it being originally derived from
another upstream kernel subsystem and whatnot.

The flipside is that losing the history makes it incrementally more
annoying for developers working on bcachefs going forward. So I can see
an argument for doing things either way in general just depending on
context, but it looks like there's precedent for either approach.
Looking back at btrfs in v2.6.29, that looks like a ~900 or so commit
history that was pulled in. bcachefs has a larger commit log (~2500+) at
this point, but if we can do whatever magic Chris referred to to try and
avoid any logistical issues for the broader kernel community, I think
that would be ideal.

BTW this is just my .02 of course, but I'm also fairly certain at least
one or two developers have looked at the git log and expressed the exact
opposite opinion expressed here: that seeing an upstream-like history is
appreciated because it reflects a sane/compatible development process.
That again isn't to say one way or the other is the right approach for a
merge, just that it seems subjective to some degree and so inevitably
there will be different opinions..

Brian