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.
next prev parent 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).