Re: [git pull] first batch of ufs fixes

[Richard Narron]

On Sun, 18 Jun 2017, Al Viro wrote:

> On Sat, Jun 17, 2017 at 03:15:48AM +0100, Al Viro wrote:
> > On Fri, Jun 16, 2017 at 07:29:00AM -0700, Richard Narron wrote:
> >
> > > The 8 patches in the ufs-fixes group were applied to Linux 4.12-rc5.
> > > They seem to work fine with the simple testing that I do.
> > >
> > > I tested all 3 BSDs, FreeBSD 11.0, OpenBSD 6.1 and NetBSD 7.1 using 2
> > > filesystems, 44bsd (ufs1) and ufs2.
> > > I found no errors doing a Linux mkdir, copy large file, BSD fsck, Linux rm
> > > large file, rmdir and BSD fsck in any of the 6 combinations.
> >
> > FWIW, with xfstests I see the following:
> > 	* _require_metadata_journaling needs to be told not to bother with
> > our ufs
> > 	* generic/{409,410,411} lack $MOUNT_OPTIONS in several invocations
> > of mount -t $FSTYP; for ufs it's obviosuly a problem.  Trivial bug in tests,
> > fixing it makes them pass.
> > 	* generic/258 (timestamp wraparound) fails; fs is left intact
>
> Trivially fixed (cast to (signed) in ufs1_read_inode(), similar to what
> other filesystems with 32bit timestamps are doing); ufs2 has no problem
> at all)
>
> > 	* generic/426 (fhandle stuff) fails and buggers the filesystem
> > Everything else passes with no fs corruption that could be detected by
> > fsck.ufs.
>
> Also trivially fixed - it's a self-inflicted wound.  Just have zero nlink in
> ufs{1,2}_read_inode() fail with -ESTALE instead of triggering ufs_error().
>
> > As for my immediate plans, I'll look into the two failing tests,
> > but any further active work on ufs will have to wait for the next
> > cycle.  It had been a fun couple of weeks, but I have more than
> > enough other stuff to deal with.  And I would still very much prefer
> > for somebody to adopt that puppy.
>
> Another piece of fun spotted: the logics for switching between two allocation
> policies when relocating a packed tail that can't be expanded in place had
> been b0rken since typo in 2.4.14.7 - switch back from OPTTIME to OPTSPACE
> had been screwed by this:
> -               usb1->fs_optim = SWAB32(UFS_OPTSPACE);
> +               usb1->fs_optim = cpu_to_fs32(sb, UFS_OPTTIME);
>
> And fragmentation levels for switching back and force really ought to be
> calculated at mount time.  Another (minor) issue is mentioned in this
> commit message from Kirck McKusick back in 1995:
>        The threshold for switching from time-space and space-time is too small
>        when minfree is 5%...so make it stay at space in this case.
> Not that minfree at 5% had been frequently seen - default has never been that
> low (back in 4.2BSD it was 10%, these days it's 8%)
>
> Resulting kernel passes xfstests clean and now I'm definitely done with UFS for
> this cycle.  Linus, in case you want to pull that sucker, pull request would
> be as below:
>
> The following changes since commit a8fad984833832d5ca11a9ed64ddc55646da30e3:
>
>  ufs_truncate_blocks(): fix the case when size is in the last direct block (2017-06-15 03:57:46 -0400)
>
> are available in the git repository at:
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git ufs-fixes
>
> for you to fetch changes up to 77e9ce327d9b607cd6e57c0f4524a654dc59c4b1:
>
>  ufs: fix the logics for tail relocation (2017-06-17 17:22:42 -0400)
>
> ----------------------------------------------------------------
> Al Viro (3):
>      fix signedness of timestamps on ufs1
>      ufs_iget(): fail with -ESTALE on deleted inode
>      ufs: fix the logics for tail relocation
>
> fs/ufs/balloc.c | 22 ++++++----------------
> fs/ufs/inode.c  | 27 +++++++++++----------------
> fs/ufs/super.c  |  9 +++++++++
> fs/ufs/ufs_fs.h |  2 ++
> 4 files changed, 28 insertions(+), 32 deletions(-)

I just tested these 3 patches along with the earlier 8 patches against 
Linux 4.12-rc5 and they look fine. All 6 test cases look good.

The ufs code is much better now than it was before all these patches.