All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Chao Yu <yuchao0@huawei.com>
Cc: fstests@vger.kernel.org, guaneryu@gmail.com,
	linux-f2fs-devel@lists.sourceforge.net, fdmanana@gmail.com
Subject: Re: [PATCH v2] generic: test i_mode recovery after power failure
Date: Wed, 6 Mar 2019 07:53:44 +1100	[thread overview]
Message-ID: <20190305205344.GC26298@dastard> (raw)
In-Reply-To: <20190305114744.129633-1-yuchao0@huawei.com>

On Tue, Mar 05, 2019 at 07:47:44PM +0800, Chao Yu wrote:
> After fsync, filesystem should guarantee inode metadata including
> permission info being persisted, so even after sudden power-cut,
> during mount, we should recover i_mode fields correctly, in order
> to not loss those meta info.
> 
> So adding this testcase to check whether generic filesystem can
> guarantee that.

Can I make a suggestion here?

I've noticed that we are getting a lot of these one-off, random
"fsync persists one specific piece of metadata in one specific case"
tests, mainly in response to some fsync bug that was found in btrfs.
This is reactive, and does not find new bugs in this area.

We are also beyond the point where the number of tests is
maintainable (e.g. to be able to make infrastructure changes), so we
really should be looking to consolidate largely similar tests into
single tests where possible.

I'd strongly suggest that a robust fsync tester should be written
for all the cases that are currently being addressed in an ad hoc
fashion. Then they can be consolidated into a single test run, and
we can get rid of all the one-off "fsync failed on this <specific
thing>" tests from the harness.

Oh, wait, we *already have that infrastructure*: src/fsync-tester.c
and generic/311.

Can we please consider rolling all of these "do something, fsync,
drop-writes, remount check" into fsync-tester.c and do the same for
all future one-off "did fsync persist X" tests?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: Chao Yu <yuchao0@huawei.com>
Cc: guaneryu@gmail.com, fdmanana@gmail.com, fstests@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [PATCH v2] generic: test i_mode recovery after power failure
Date: Wed, 6 Mar 2019 07:53:44 +1100	[thread overview]
Message-ID: <20190305205344.GC26298@dastard> (raw)
In-Reply-To: <20190305114744.129633-1-yuchao0@huawei.com>

On Tue, Mar 05, 2019 at 07:47:44PM +0800, Chao Yu wrote:
> After fsync, filesystem should guarantee inode metadata including
> permission info being persisted, so even after sudden power-cut,
> during mount, we should recover i_mode fields correctly, in order
> to not loss those meta info.
> 
> So adding this testcase to check whether generic filesystem can
> guarantee that.

Can I make a suggestion here?

I've noticed that we are getting a lot of these one-off, random
"fsync persists one specific piece of metadata in one specific case"
tests, mainly in response to some fsync bug that was found in btrfs.
This is reactive, and does not find new bugs in this area.

We are also beyond the point where the number of tests is
maintainable (e.g. to be able to make infrastructure changes), so we
really should be looking to consolidate largely similar tests into
single tests where possible.

I'd strongly suggest that a robust fsync tester should be written
for all the cases that are currently being addressed in an ad hoc
fashion. Then they can be consolidated into a single test run, and
we can get rid of all the one-off "fsync failed on this <specific
thing>" tests from the harness.

Oh, wait, we *already have that infrastructure*: src/fsync-tester.c
and generic/311.

Can we please consider rolling all of these "do something, fsync,
drop-writes, remount check" into fsync-tester.c and do the same for
all future one-off "did fsync persist X" tests?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  parent reply	other threads:[~2019-03-05 20:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-05 11:47 [PATCH v2] generic: test i_mode recovery after power failure Chao Yu
2019-03-05 11:47 ` Chao Yu
2019-03-05 14:41 ` Filipe Manana
2019-03-05 14:41   ` Filipe Manana
2019-03-05 20:53 ` Dave Chinner [this message]
2019-03-05 20:53   ` Dave Chinner
2019-03-06  2:29   ` Chao Yu
2019-03-06  2:29     ` Chao Yu
2019-03-06  5:00     ` Dave Chinner
2019-03-06  5:00       ` Dave Chinner
2019-03-06  7:44       ` Amir Goldstein
2019-03-06  7:44         ` Amir Goldstein
2019-03-06 22:12         ` Dave Chinner
2019-03-06 22:12           ` Dave Chinner
2019-03-07  7:12           ` Amir Goldstein
2019-03-07  7:12             ` Amir Goldstein
2019-03-07 20:22             ` Dave Chinner
2019-03-07 20:22               ` Dave Chinner
2019-03-07 20:42               ` [f2fs-dev] " Jayashree Mohan
2019-03-07 20:42                 ` Jayashree Mohan
2019-03-09 10:15   ` Eryu Guan
2019-03-09 10:15     ` Eryu Guan

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=20190305205344.GC26298@dastard \
    --to=david@fromorbit.com \
    --cc=fdmanana@gmail.com \
    --cc=fstests@vger.kernel.org \
    --cc=guaneryu@gmail.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=yuchao0@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.