Message-ID: <20170617021548.GR31671@ZenIV.linux.org.uk>
Date:   Sat, 17 Jun 2017 03:15:48 +0100
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Richard Narron <comet.berkeley@...il.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [git pull] first batch of ufs fixes

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
	* generic/426 (fhandle stuff) fails and buggers the filesystem
Everything else passes with no fs corruption that could be detected by
fsck.ufs.  Note that crash simulation tests are *not* run - no point
even trying.  I tried several of those with -o sync; they seem to pass,
but there's free blocks, etc. stats summaries being slightly incorrect.
Inevitable, AFAICS.

> Doing a "df" on BSD and Linux now match on the counts including the
> "Available" counts.
> 
> It might be worth testing with ufs filesystems using softdep and/or
> journaling.  Should the Linux mount command reject such filesystems?

Depends...  Do you mean rejecting mount of fs with pending log replay?

> Now that ufs write access is working more or less, we're dangerous.

*snort*
It's marked experimental for a very good reason.  The stuff in #ufs-fixes
deals with fairly obvious bugs and with those the thing is reasonably
well-behaved on xfstests, but it needs a lot more testing (and probably
somebody doing journalling side of things as well) before it can be
considered fully safe for sharing with *BSD.

And I'm still not entirely convinced that mixing tail-unpacking, truncate
and mmap works correctly.  FWIW, switch to iomap-based approach would
probably deal with all of that nicely, but that's well beyond "obvious
bugfixes"; definitely not this cycle fodder.

I'm not even sure if #ufs-fixes is OK for this stage in the cycle - it
is local to fs/ufs, doesn't affect anything else and fixes fairly
obvious bugs, but we are nearly at -rc6.  Pull request for that would
be as below, but it's really up to Linus (and Evgeniy, if he is not
completely MIA) whether to pull it now or wait until the next window.
Linus, what do you think of that?  I'm honestly not sure...

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.

----
Fix assorted ufs bugs; a couple of deadlocks, fs corruption in truncate(),
oopsen on tail unpacking and truncate when racing with vmscan, mild fs
corruption (free blocks stats summary buggered, *BSD fsck would
complain and fix), several instances of broken logics around reserved
blocks (starting with "check almost never triggers when it should" and
then there are issues with sufficiently large UFS2).

The following changes since commit 67a70017fa0a152657bc7e337e69bb9c9f5549bf:

  ufs: we need to sync inode before freeing it (2017-06-10 12:02:28 -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 a8fad984833832d5ca11a9ed64ddc55646da30e3:

  ufs_truncate_blocks(): fix the case when size is in the last direct block (2017-06-15 03:57:46 -0400)

----------------------------------------------------------------
Al Viro (8):
      ufs: fix logics in "ufs: make fsck -f happy"
      ufs: make ufs_freespace() return signed
      ufs: fix reserved blocks check
      ufs: fix s_size/s_dsize users
      ufs_get_locked_page(): make sure we have buffer_heads
      ufs: avoid grabbing ->truncate_mutex if possible
      ufs: more deadlock prevention on tail unpacking
      ufs_truncate_blocks(): fix the case when size is in the last direct block

 fs/ufs/balloc.c | 22 ++++++++++++--------
 fs/ufs/inode.c  | 47 ++++++++++++++++++++++++++++--------------
 fs/ufs/super.c  | 64 +++++++++++++++++++++++++++++++++++----------------------
 fs/ufs/ufs_fs.h |  7 +++----
 fs/ufs/util.c   | 17 ++++++++-------
 fs/ufs/util.h   |  9 ++------
 6 files changed, 98 insertions(+), 68 deletions(-)

Powered by blists - more mailing lists