Message-ID: <20260212164402.tbjcalfmeq6jfwum@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com>
Date: Fri, 13 Feb 2026 00:44:02 +0800
From: Zorro Lang <zlang@...nel.org>
To: Jan Kara <jack@...e.cz>
Cc: Zorro Lang <zlang@...hat.com>, fstests@...r.kernel.org,
	linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org
Subject: Re: [PATCH 0/4] Avoid failing shutdown tests without a journal

On Thu, Feb 12, 2026 at 11:41:59AM +0100, Jan Kara wrote:
> On Thu 12-02-26 16:40:50, Zorro Lang wrote:
> > On Tue, Feb 10, 2026 at 12:20:17PM +0100, Jan Kara wrote:
> > > Hello,
> > > 
> > > this patch series adds requirement for metadata journalling to couple
> > > of tests using filesystem shutdown. After shutdown a filesystem without
> > > a journal is not guaranteed to be consistent and thus tests often fail.
> > 
> > Hi Jan,
> > 
> > This patchset makes sense to me, thanks for fixing them :)
> > 
> > Since you brought this up, I just tried to check all cases using _require_scratch_shutdown
> > but lack _require_metadata_journaling, I got this:
> > 
> > $ for f in `grep -rsnl _require_scratch_shutdown tests/`;do grep -q _require_metadata_journaling $f || echo $f;done
> > tests/ext4/051       <=== fixed by this patchset
> > tests/generic/050
> > tests/generic/461
> > tests/generic/474
> > tests/generic/536
> > tests/generic/599
> > tests/generic/622
> > tests/generic/635    <=== fixed by this patchset
> > tests/generic/646    <=== fixed by this patchset
> > tests/generic/705    <=== fixed by this patchset
> > tests/generic/722
> > tests/generic/730
> > tests/generic/737
> > tests/generic/775
> > tests/generic/778
> > tests/overlay/078
> > tests/overlay/087
> > tests/xfs/270
> > tests/xfs/546
> > 
> > g/050 tests ro mount, so it might not need _require_metadata_journaling.
> 

Hi Jan,

Thanks for your detailed response.

> Yes, g/050 checks using _has_metadata_journaling and treats the fs
> accordingly.

I didn't notice that line, thanks.

> 
> > g/461 doesn't care the fs consistency, so ignore it too.
> 
> Ack.
> 
> > g/730 looks like doesn't need _require_metadata_journaling.
> 
> Ack.
> 
> > overlay/087 looks like can ignore _require_metadata_journaling.
> 
> I think this actually needs it since we shutdown the fs and then mount it
> again which may fail with inconsistent fs.

Thanks for the reminder, I totally overlooked the sync call. You're right, even
aside from the journal, sync effectively help filesystem to be a consistent
state (if no any hardware crashes during the sync running)

> 
> > Others, include g/474, g/536, g/599, g/622, g/722, g/737, g/775, g/778,
> > overlay/078
> > look like all need a journal fs.
> 
> g/474 seems good to test without metadata journalling it does syncfs and
> then shutdown. In that case the data checksums should be valid even without
> journalling.

Sure.

> 
> g/599 should be also fine - we shutdown after remounting ro so that should
> be working even without journalling.

Oh, there's "_scratch_remount ro" before shutdown, sure.

> 
> g/622 - some bits should work even without journalling but for now I guess
> we should just require it for simplicity.
> 
> g/737 - uses O_SYNC - should work even without journalling (perhaps the
> test should be extended to fsync also the parent dir for complete correctness).
> 
> g/775 - tests atomic writes - should work even without journalling if the
> fs claims to support atomic writes
> 
> g/778 - ditto
> 
> overlay/078 - this fsyncs the file before shutdown so it should work even
> without journalling (but might need to fsync the parent dir for complete
> correctness).

I'm not sure we can arbitrarily add more sync calls before or after a specific
fsync step. Doing so might break the original intent of the test. So I think
we can keep it, and see if anyone complains or hits a bug later :)

> 
> > About x/270 and x/546, if we don't suppose other fs would like to run
> > xfs cases, then xfs always support journal.
> 
> Correct.
> 
> > I initially considered calling _require_metadata_journaling directly inside
> > _require_scratch_shutdown. However, I decided against it because some cases might
> > only need the shutdown ioctl and don't strictly require a journal.
> 
> Absolutely. I think they should stay separate.
> 
> So to summarize I think we should still add _require_metadata_journaling to:
> 
> overlay/087
> g/536
> g/622
> g/722

Agree :)

> 
> and we might add fsync of parent directory before shutdown to g/737 and
> overlay/078. Does this sound good?

I'm concerned that adding broader sync or fsync operations might interfere with the
test's original intent. We should probably evaluate the impact further. Alternatively,
we could simply use _require_metadata_journaling to ensure we at least keep the
coverage for the original bug :)

Any more ideas please feel free to tell me.

Thanks,
Zorro

> 
> 								Honza
> -- 
> Jan Kara <jack@...e.com>
> SUSE Labs, CR
> 

Powered by blists - more mailing lists