Re: [PATCHSET v2] xfs: proposed bug fixes for 6.13

[Bill O'Donnell <bodonnel@redhat.com>]

On Wed, Dec 04, 2024 at 10:42:43PM -0800, Darrick J. Wong wrote:
> On Wed, Dec 04, 2024 at 07:26:54PM -0600, Bill O'Donnell wrote:
> > On Tue, Dec 03, 2024 at 07:02:23PM -0800, Darrick J. Wong wrote:
> > > Hi all,
> > > 
> > > Here are even more bugfixes for 6.13 that have been accumulating since
> > > 6.12 was released.
> > > 
> > > If you're going to start using this code, I strongly recommend pulling
> > > from my git trees, which are linked below.
> > > 
> > > With a bit of luck, this should all go splendidly.
> > > Comments and questions are, as always, welcome.
> > > 
> > > --D
> > 
> > Hi Darrick-
> > 
> > I must ask, why are these constant bug fixes and fixes for fixes, and
> > fixes for fixes for fixes often appearing? It's worrying that xfs is
> 
> Roughly speaking, the ~35 bugfixes can be split into three categories:
> 
> 1) Our vaunted^Wshitty review process didn't catch various coding bugs,
> and testing didn't trip over them until I started (ab)using precommit
> hooks for spot checking of inode/dquot/buffer log items.

You give little time for the review process.

> 
> 2) Most of the metadir/rtgroups fixes are for things that hch reworked
> towards the end of the six years the patchset has been under
> development, and that introduced bugs.  Did it make things easier for a
> second person to understand?  Yes.

No.

> 
> 3) The rest are mostly cases of the authors not fully understanding the
> subtleties of that which they were constructing (myself included!) and
> lucking out that the errors cancelled each other out until someone
> started wanting to use that code for a slightly different purpose, which
> wouldn't be possible until the bug finally got fixed.
> 
> 4) The dquot buffer changes have been a known problem since dchinner
> decided that RMW cycles in the AIL with inode buffers was having very
> bad effects on reclaim performance.  Nobody stepped up to convert dquots
> (even though I noted this at the time) so here I am years later because
> the mm got pissy at us in 6.12.
> 
> 5) XFS lit up a lot of new functionality this year, which means the code
> is ripe with bugfixing opportunities where cognitive friction comes into
> play.

I call bullshit. You guys are fast and loose with your patches. Giving
little time for review and soaking.

> 
> > becoming rather dodgy these days. Do things need to be this
> > complicated?
> 
> Yeah, they do.  We left behind the kindly old world where people didn't
> feed computers fuzzed datafiles and nobody got fired for a computer
> crashing periodically.  Nowadays it seems that everything has to be
> bulletproofed AND fast. :(

Cop-out answer.

> 
> --D
> 
> > -Bill
> > 
> > 
> > > 
> > > kernel git tree:
> > > https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=proposed-fixes-6.13
> > > ---
> > > Commits in this patchset:
> > >  * xfs: don't move nondir/nonreg temporary repair files to the metadir namespace
> > >  * xfs: don't crash on corrupt /quotas dirent
> > >  * xfs: check pre-metadir fields correctly
> > >  * xfs: fix zero byte checking in the superblock scrubber
> > >  * xfs: return from xfs_symlink_verify early on V4 filesystems
> > >  * xfs: port xfs_ioc_start_commit to multigrain timestamps
> > > ---
> > >  fs/xfs/libxfs/xfs_symlink_remote.c |    4 ++
> > >  fs/xfs/scrub/agheader.c            |   66 ++++++++++++++++++++++++++++--------
> > >  fs/xfs/scrub/tempfile.c            |    3 ++
> > >  fs/xfs/xfs_exchrange.c             |   14 ++++----
> > >  fs/xfs/xfs_qm.c                    |    7 ++++
> > >  5 files changed, 71 insertions(+), 23 deletions(-)
> > > 
> > > 
> > 
> > 
>