linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Narron <comet.berkeley@gmail.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [git pull] first batch of ufs fixes
Date: Sun, 18 Jun 2017 13:45:41 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LNX.2.21.1706181340260.2386@joy.test> (raw)
In-Reply-To: <20170618010923.GX31671@ZenIV.linux.org.uk>

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.

  reply	other threads:[~2017-06-18 20:45 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-09 21:38 [git pull] first batch of ufs fixes Al Viro
2017-06-10 13:03 ` Richard Narron
2017-06-10 16:07   ` Al Viro
2017-06-10 18:08     ` Al Viro
2017-06-10 18:12       ` Linus Torvalds
2017-06-11 19:47       ` Richard Narron
2017-06-11 21:30         ` Al Viro
2017-06-12  6:14         ` Al Viro
2017-06-13  0:54           ` Richard Narron
2017-06-13  1:43             ` Al Viro
2017-06-13 21:56               ` Richard Narron
2017-06-14  7:11                 ` Al Viro
2017-06-14 20:33                   ` Richard Narron
2017-06-15  8:00                   ` Al Viro
2017-06-16 14:29                     ` Richard Narron
2017-06-17  2:15                       ` Al Viro
2017-06-18  1:09                         ` Al Viro
2017-06-18 20:45                           ` Richard Narron [this message]
2017-06-20  5:17                           ` [git pull] " Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LNX.2.21.1706181340260.2386@joy.test \
    --to=comet.berkeley@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).