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

Bill O'Donnell
2024-12-05T06:52:36Z

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