linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] Why generic/073 is generic but not btrfs specific?
@ 2025-11-06 20:40 Viacheslav Dubeyko
  2025-11-06 20:58 ` Qu Wenruo
  0 siblings, 1 reply; 9+ messages in thread
From: Viacheslav Dubeyko @ 2025-11-06 20:40 UTC (permalink / raw)
  To: fstests@vger.kernel.org
  Cc: linux-btrfs@vger.kernel.org, glaubitz@physik.fu-berlin.de,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org

Hello,

Running generic/073 for the case of HFS+ finishes with volume corruption:

sudo ./check generic/073
FSTYP -- hfsplus
PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.17.0-rc1+ #4 SMP PREEMPT_DYNAMIC
Wed Oct 1 15:02:44 PDT 2025
MKFS_OPTIONS -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

generic/073 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent
(see XFSTESTS-2/xfstests-dev/results//generic/073.full for details)

Ran: generic/073
Failures: generic/073
Failed 1 of 1 tests

sudo fsck.hfsplus -d /dev/loop51
** /dev/loop51
Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K.
Executing fsck_hfs (version 540.1-Linux).
** Checking non-journaled HFS Plus Volume.
The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
** Checking multi-linked files.
** Checking catalog hierarchy.
Invalid directory item count
(It should be 1 instead of 0)
** Checking extended attributes file.
** Checking volume bitmap.
** Checking volume information.
Verify Status: VIStat = 0x0000, ABTStat = 0x0000 EBTStat = 0x0000
CBTStat = 0x0000 CatStat = 0x00004000
** Repairing volume.
** Rechecking volume.
** Checking non-journaled HFS Plus Volume.
The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
** Checking multi-linked files.
** Checking catalog hierarchy.
** Checking extended attributes file.
** Checking volume bitmap.
** Checking volume information.
** The volume untitled was repaired successfully.

Initially, I considered that something is wrong with HFS+ driver logic. But
after testing and debugging the issue, I believe that HFS+ logic is correct.

As far as I can see, the generic/073 is checking specific btrfs related case:

# Test file A fsync after moving one other unrelated file B between directories
# and fsyncing B's old parent directory before fsyncing the file A. Check that
# after a crash all the file A data we fsynced is available.
#
# This test is motivated by an issue discovered in btrfs which caused the file
# data to be lost (despite fsync returning success to user space). That btrfs
# bug was fixed by the following linux kernel patch:
#
#   Btrfs: fix data loss in the fast fsync path

The test is doing these steps on final phase:

mv $SCRATCH_MNT/testdir_1/bar $SCRATCH_MNT/testdir_2/bar
$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir_1
$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/foo

So, we move file bar from testdir_1 into testdir_2 folder. It means that HFS+
logic decrements the number of entries in testdir_1 and increments number of
entries in testdir_2. Finally, we do fsync only for testdir_1 and foo but not
for testdir_2. As a result, this is the reason why fsck.hfsplus detects the
volume corruption afterwards. As far as I can see, the HFS+ driver behavior is
completely correct and nothing needs to be done for fixing in HFS+ logic here.

But what could be the proper solution? Should generic/073 be excluded from
HFS/HFS+ xfstests run? Or, maybe, generic/073 needs to be btrfs specific? Am I
missing something here?

Thanks,
Slava.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Why generic/073 is generic but not btrfs specific?
  2025-11-06 20:40 [RFC] Why generic/073 is generic but not btrfs specific? Viacheslav Dubeyko
@ 2025-11-06 20:58 ` Qu Wenruo
  2025-11-06 21:11   ` Viacheslav Dubeyko
  0 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2025-11-06 20:58 UTC (permalink / raw)
  To: Viacheslav Dubeyko, fstests@vger.kernel.org
  Cc: linux-btrfs@vger.kernel.org, glaubitz@physik.fu-berlin.de,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org



在 2025/11/7 07:10, Viacheslav Dubeyko 写道:
> Hello,
> 
> Running generic/073 for the case of HFS+ finishes with volume corruption:
> 
> sudo ./check generic/073
> FSTYP -- hfsplus
> PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.17.0-rc1+ #4 SMP PREEMPT_DYNAMIC
> Wed Oct 1 15:02:44 PDT 2025
> MKFS_OPTIONS -- /dev/loop51
> MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
> 
> generic/073 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent
> (see XFSTESTS-2/xfstests-dev/results//generic/073.full for details)
> 
> Ran: generic/073
> Failures: generic/073
> Failed 1 of 1 tests
> 
> sudo fsck.hfsplus -d /dev/loop51
> ** /dev/loop51
> Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K.
> Executing fsck_hfs (version 540.1-Linux).
> ** Checking non-journaled HFS Plus Volume.
> The volume name is untitled
> ** Checking extents overflow file.
> ** Checking catalog file.
> ** Checking multi-linked files.
> ** Checking catalog hierarchy.
> Invalid directory item count
> (It should be 1 instead of 0)
> ** Checking extended attributes file.
> ** Checking volume bitmap.
> ** Checking volume information.
> Verify Status: VIStat = 0x0000, ABTStat = 0x0000 EBTStat = 0x0000
> CBTStat = 0x0000 CatStat = 0x00004000
> ** Repairing volume.
> ** Rechecking volume.
> ** Checking non-journaled HFS Plus Volume.
> The volume name is untitled
> ** Checking extents overflow file.
> ** Checking catalog file.
> ** Checking multi-linked files.
> ** Checking catalog hierarchy.
> ** Checking extended attributes file.
> ** Checking volume bitmap.
> ** Checking volume information.
> ** The volume untitled was repaired successfully.
> 
> Initially, I considered that something is wrong with HFS+ driver logic. But
> after testing and debugging the issue, I believe that HFS+ logic is correct.
> 
> As far as I can see, the generic/073 is checking specific btrfs related case:
> 
> # Test file A fsync after moving one other unrelated file B between directories
> # and fsyncing B's old parent directory before fsyncing the file A. Check that
> # after a crash all the file A data we fsynced is available.
> #
> # This test is motivated by an issue discovered in btrfs which caused the file
> # data to be lost (despite fsync returning success to user space). That btrfs
> # bug was fixed by the following linux kernel patch:
> #
> #   Btrfs: fix data loss in the fast fsync path
> 
> The test is doing these steps on final phase:
> 
> mv $SCRATCH_MNT/testdir_1/bar $SCRATCH_MNT/testdir_2/bar
> $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir_1
> $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/foo
> 
> So, we move file bar from testdir_1 into testdir_2 folder. It means that HFS+
> logic decrements the number of entries in testdir_1 and increments number of
> entries in testdir_2. Finally, we do fsync only for testdir_1 and foo but not
> for testdir_2.

If the fs is using journal, shouldn't the increments on the testdir_2 
already be journaled? Thus after a power loss, the increments on 
testdir_2/bar should be replayed thus the end user should still see that 
inode.

To me this looks like a bug in HFS logic where something is not properly 
journaled (the increment on the testdir_2/bar).


Finally, if you're asking an end user that if it is acceptable that 
after moving an inode and fsync the old directory, the inode is no 
longer reachable, I'm pretty sure no end user will think it's acceptable.

> As a result, this is the reason why fsck.hfsplus detects the
> volume corruption afterwards. As far as I can see, the HFS+ driver behavior is
> completely correct and nothing needs to be done for fixing in HFS+ logic here.

Then I guess you may also want to ask why EXT4/XFS/Btrfs/F2fs all pass 
the test case.

Thanks,
Qu

> 
> But what could be the proper solution? Should generic/073 be excluded from
> HFS/HFS+ xfstests run? Or, maybe, generic/073 needs to be btrfs specific? Am I
> missing something here?
> 
> Thanks,
> Slava.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC] Why generic/073 is generic but not btrfs specific?
  2025-11-06 20:58 ` Qu Wenruo
@ 2025-11-06 21:11   ` Viacheslav Dubeyko
  2025-11-06 21:32     ` Qu Wenruo
  0 siblings, 1 reply; 9+ messages in thread
From: Viacheslav Dubeyko @ 2025-11-06 21:11 UTC (permalink / raw)
  To: fstests@vger.kernel.org, wqu@suse.com
  Cc: linux-btrfs@vger.kernel.org, glaubitz@physik.fu-berlin.de,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org

On Fri, 2025-11-07 at 07:28 +1030, Qu Wenruo wrote:
> 
> 在 2025/11/7 07:10, Viacheslav Dubeyko 写道:
> > Hello,
> > 
> > Running generic/073 for the case of HFS+ finishes with volume corruption:
> > 
> > sudo ./check generic/073
> > FSTYP -- hfsplus
> > PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.17.0-rc1+ #4 SMP PREEMPT_DYNAMIC
> > Wed Oct 1 15:02:44 PDT 2025
> > MKFS_OPTIONS -- /dev/loop51
> > MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
> > 
> > generic/073 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent
> > (see XFSTESTS-2/xfstests-dev/results//generic/073.full for details)
> > 
> > Ran: generic/073
> > Failures: generic/073
> > Failed 1 of 1 tests
> > 
> > sudo fsck.hfsplus -d /dev/loop51
> > ** /dev/loop51
> > Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K.
> > Executing fsck_hfs (version 540.1-Linux).
> > ** Checking non-journaled HFS Plus Volume.
> > The volume name is untitled
> > ** Checking extents overflow file.
> > ** Checking catalog file.
> > ** Checking multi-linked files.
> > ** Checking catalog hierarchy.
> > Invalid directory item count
> > (It should be 1 instead of 0)
> > ** Checking extended attributes file.
> > ** Checking volume bitmap.
> > ** Checking volume information.
> > Verify Status: VIStat = 0x0000, ABTStat = 0x0000 EBTStat = 0x0000
> > CBTStat = 0x0000 CatStat = 0x00004000
> > ** Repairing volume.
> > ** Rechecking volume.
> > ** Checking non-journaled HFS Plus Volume.
> > The volume name is untitled
> > ** Checking extents overflow file.
> > ** Checking catalog file.
> > ** Checking multi-linked files.
> > ** Checking catalog hierarchy.
> > ** Checking extended attributes file.
> > ** Checking volume bitmap.
> > ** Checking volume information.
> > ** The volume untitled was repaired successfully.
> > 
> > Initially, I considered that something is wrong with HFS+ driver logic. But
> > after testing and debugging the issue, I believe that HFS+ logic is correct.
> > 
> > As far as I can see, the generic/073 is checking specific btrfs related case:
> > 
> > # Test file A fsync after moving one other unrelated file B between directories
> > # and fsyncing B's old parent directory before fsyncing the file A. Check that
> > # after a crash all the file A data we fsynced is available.
> > #
> > # This test is motivated by an issue discovered in btrfs which caused the file
> > # data to be lost (despite fsync returning success to user space). That btrfs
> > # bug was fixed by the following linux kernel patch:
> > #
> > #   Btrfs: fix data loss in the fast fsync path
> > 
> > The test is doing these steps on final phase:
> > 
> > mv $SCRATCH_MNT/testdir_1/bar $SCRATCH_MNT/testdir_2/bar
> > $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir_1
> > $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/foo
> > 
> > So, we move file bar from testdir_1 into testdir_2 folder. It means that HFS+
> > logic decrements the number of entries in testdir_1 and increments number of
> > entries in testdir_2. Finally, we do fsync only for testdir_1 and foo but not
> > for testdir_2.
> 
> If the fs is using journal, shouldn't the increments on the testdir_2 
> already be journaled? Thus after a power loss, the increments on 
> testdir_2/bar should be replayed thus the end user should still see that 
> inode.
> 

Technically speaking, HFS+ is journaling file system in Apple implementation.
But we don't have this functionality implemented and fully supported on Linux
kernel side. Potentially, it can be done but currently we haven't such
functionality yet. So, HFS/HFS+ doesn't use journaling on Linux kernel side  and
no journal replay could happen. :)

> To me this looks like a bug in HFS logic where something is not properly 
> journaled (the increment on the testdir_2/bar).
> 
> 

This statement could be correct if we will support journaling for HFS+. But HFS
never supported the journaling technology.

> Finally, if you're asking an end user that if it is acceptable that 
> after moving an inode and fsync the old directory, the inode is no 
> longer reachable, I'm pretty sure no end user will think it's acceptable.
> 

As far as I can see, the fsck only corrects the number of entries for testdir_2
folder. The rest is pretty OK.

> > As a result, this is the reason why fsck.hfsplus detects the
> > volume corruption afterwards. As far as I can see, the HFS+ driver behavior is
> > completely correct and nothing needs to be done for fixing in HFS+ logic here.
> 
> Then I guess you may also want to ask why EXT4/XFS/Btrfs/F2fs all pass 
> the test case.
> 

So, ext4 and xfs support journaling. Maybe, it's interesting to check what is
going on with F2FS, FAT, and other file system that hasn't journaling support.

Thanks,
Slava.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Why generic/073 is generic but not btrfs specific?
  2025-11-06 21:11   ` Viacheslav Dubeyko
@ 2025-11-06 21:32     ` Qu Wenruo
  2025-11-06 22:29       ` Viacheslav Dubeyko
  0 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2025-11-06 21:32 UTC (permalink / raw)
  To: Viacheslav Dubeyko, fstests@vger.kernel.org
  Cc: linux-btrfs@vger.kernel.org, glaubitz@physik.fu-berlin.de,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org



在 2025/11/7 07:41, Viacheslav Dubeyko 写道:
> On Fri, 2025-11-07 at 07:28 +1030, Qu Wenruo wrote:
>>
>> 在 2025/11/7 07:10, Viacheslav Dubeyko 写道:
>>> Hello,
>>>
>>> Running generic/073 for the case of HFS+ finishes with volume corruption:
>>>
>>> sudo ./check generic/073
>>> FSTYP -- hfsplus
>>> PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.17.0-rc1+ #4 SMP PREEMPT_DYNAMIC
>>> Wed Oct 1 15:02:44 PDT 2025
>>> MKFS_OPTIONS -- /dev/loop51
>>> MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
>>>
>>> generic/073 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent
>>> (see XFSTESTS-2/xfstests-dev/results//generic/073.full for details)
>>>
>>> Ran: generic/073
>>> Failures: generic/073
>>> Failed 1 of 1 tests
>>>
>>> sudo fsck.hfsplus -d /dev/loop51
>>> ** /dev/loop51
>>> Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K.
>>> Executing fsck_hfs (version 540.1-Linux).
>>> ** Checking non-journaled HFS Plus Volume.
>>> The volume name is untitled
>>> ** Checking extents overflow file.
>>> ** Checking catalog file.
>>> ** Checking multi-linked files.
>>> ** Checking catalog hierarchy.
>>> Invalid directory item count
>>> (It should be 1 instead of 0)
>>> ** Checking extended attributes file.
>>> ** Checking volume bitmap.
>>> ** Checking volume information.
>>> Verify Status: VIStat = 0x0000, ABTStat = 0x0000 EBTStat = 0x0000
>>> CBTStat = 0x0000 CatStat = 0x00004000
>>> ** Repairing volume.
>>> ** Rechecking volume.
>>> ** Checking non-journaled HFS Plus Volume.
>>> The volume name is untitled
>>> ** Checking extents overflow file.
>>> ** Checking catalog file.
>>> ** Checking multi-linked files.
>>> ** Checking catalog hierarchy.
>>> ** Checking extended attributes file.
>>> ** Checking volume bitmap.
>>> ** Checking volume information.
>>> ** The volume untitled was repaired successfully.
>>>
>>> Initially, I considered that something is wrong with HFS+ driver logic. But
>>> after testing and debugging the issue, I believe that HFS+ logic is correct.
>>>
>>> As far as I can see, the generic/073 is checking specific btrfs related case:
>>>
>>> # Test file A fsync after moving one other unrelated file B between directories
>>> # and fsyncing B's old parent directory before fsyncing the file A. Check that
>>> # after a crash all the file A data we fsynced is available.
>>> #
>>> # This test is motivated by an issue discovered in btrfs which caused the file
>>> # data to be lost (despite fsync returning success to user space). That btrfs
>>> # bug was fixed by the following linux kernel patch:
>>> #
>>> #   Btrfs: fix data loss in the fast fsync path
>>>
>>> The test is doing these steps on final phase:
>>>
>>> mv $SCRATCH_MNT/testdir_1/bar $SCRATCH_MNT/testdir_2/bar
>>> $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir_1
>>> $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/foo
>>>
>>> So, we move file bar from testdir_1 into testdir_2 folder. It means that HFS+
>>> logic decrements the number of entries in testdir_1 and increments number of
>>> entries in testdir_2. Finally, we do fsync only for testdir_1 and foo but not
>>> for testdir_2.
>>
>> If the fs is using journal, shouldn't the increments on the testdir_2
>> already be journaled? Thus after a power loss, the increments on
>> testdir_2/bar should be replayed thus the end user should still see that
>> inode.
>>
> 
> Technically speaking, HFS+ is journaling file system in Apple implementation.
> But we don't have this functionality implemented and fully supported on Linux
> kernel side. Potentially, it can be done but currently we haven't such
> functionality yet. So, HFS/HFS+ doesn't use journaling on Linux kernel side  and
> no journal replay could happen. :)

That's fine, btrfs doesn't support (traditional) journal either, since 
its metadata is already fully COW protected.

The journal-like part is called log-tree, which is only utilized for 
speeding up fsync() so no full fs sync is needed (which is super 
expensive for btrfs)

That's the reason why btrfs' fsync() is super complex and Filipe spent 
tons of his time on this field, and it is btrfs that exposed this problem.


But if HFS+ on linux doesn't support metadata journal, I'm afraid you 
will need to go the hard path just like btrfs, to manually check if an 
inode needs its parent inodes updated during fsync.
> 
>> To me this looks like a bug in HFS logic where something is not properly
>> journaled (the increment on the testdir_2/bar).
>>
>>
> 
> This statement could be correct if we will support journaling for HFS+. But HFS
> never supported the journaling technology.
> 
>> Finally, if you're asking an end user that if it is acceptable that
>> after moving an inode and fsync the old directory, the inode is no
>> longer reachable, I'm pretty sure no end user will think it's acceptable.
>>
> 
> As far as I can see, the fsck only corrects the number of entries for testdir_2
> folder. The rest is pretty OK.

So it means the nlink number on the child inode is correct. I guess 
since testdir_2 is not the directory where fsync is called, thus its 
number of references is not updated.

With journal it will be much easier, but without it, you have to iterate 
through the changed inode backrefs, and update the involved directories 
one by one during fsync(), just like btrfs...

> 
>>> As a result, this is the reason why fsck.hfsplus detects the
>>> volume corruption afterwards. As far as I can see, the HFS+ driver behavior is
>>> completely correct and nothing needs to be done for fixing in HFS+ logic here.
>>
>> Then I guess you may also want to ask why EXT4/XFS/Btrfs/F2fs all pass
>> the test case.
>>
> 
> So, ext4 and xfs support journaling. Maybe, it's interesting to check what is
> going on with F2FS, FAT, and other file system that hasn't journaling support.

IIRC F2FS also support journals.

For VFAT the test case is rejected due to the lack of journal support.

On the other hand, if HFS+ doesn't support journal at all, you may want 
to add hfs/hfs+ to _has_metadata_journaling(), so that the test case 
will be skipped.

But still, since this test case already leads to fsck error reports, it 
is worthy some improvement to avoid such problem.

Thanks,
Qu

> 
> Thanks,
> Slava.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC] Why generic/073 is generic but not btrfs specific?
  2025-11-06 21:32     ` Qu Wenruo
@ 2025-11-06 22:29       ` Viacheslav Dubeyko
  2025-11-08 14:01         ` Theodore Ts'o
  0 siblings, 1 reply; 9+ messages in thread
From: Viacheslav Dubeyko @ 2025-11-06 22:29 UTC (permalink / raw)
  To: fstests@vger.kernel.org, wqu@suse.com
  Cc: linux-btrfs@vger.kernel.org, glaubitz@physik.fu-berlin.de,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org

On Fri, 2025-11-07 at 08:02 +1030, Qu Wenruo wrote:
> 
> 在 2025/11/7 07:41, Viacheslav Dubeyko 写道:
> > On Fri, 2025-11-07 at 07:28 +1030, Qu Wenruo wrote:
> > > 
> > > 在 2025/11/7 07:10, Viacheslav Dubeyko 写道:
> > > > Hello,
> > > > 
> > > > Running generic/073 for the case of HFS+ finishes with volume corruption:
> > > > 
> > > > sudo ./check generic/073
> > > > FSTYP -- hfsplus
> > > > PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.17.0-rc1+ #4 SMP PREEMPT_DYNAMIC
> > > > Wed Oct 1 15:02:44 PDT 2025
> > > > MKFS_OPTIONS -- /dev/loop51
> > > > MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
> > > > 
> > > > generic/073 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent
> > > > (see XFSTESTS-2/xfstests-dev/results//generic/073.full for details)
> > > > 
> > > > Ran: generic/073
> > > > Failures: generic/073
> > > > Failed 1 of 1 tests
> > > > 
> > > > sudo fsck.hfsplus -d /dev/loop51
> > > > ** /dev/loop51
> > > > Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K.
> > > > Executing fsck_hfs (version 540.1-Linux).
> > > > ** Checking non-journaled HFS Plus Volume.
> > > > The volume name is untitled
> > > > ** Checking extents overflow file.
> > > > ** Checking catalog file.
> > > > ** Checking multi-linked files.
> > > > ** Checking catalog hierarchy.
> > > > Invalid directory item count
> > > > (It should be 1 instead of 0)
> > > > ** Checking extended attributes file.
> > > > ** Checking volume bitmap.
> > > > ** Checking volume information.
> > > > Verify Status: VIStat = 0x0000, ABTStat = 0x0000 EBTStat = 0x0000
> > > > CBTStat = 0x0000 CatStat = 0x00004000
> > > > ** Repairing volume.
> > > > ** Rechecking volume.
> > > > ** Checking non-journaled HFS Plus Volume.
> > > > The volume name is untitled
> > > > ** Checking extents overflow file.
> > > > ** Checking catalog file.
> > > > ** Checking multi-linked files.
> > > > ** Checking catalog hierarchy.
> > > > ** Checking extended attributes file.
> > > > ** Checking volume bitmap.
> > > > ** Checking volume information.
> > > > ** The volume untitled was repaired successfully.
> > > > 
> > > > Initially, I considered that something is wrong with HFS+ driver logic. But
> > > > after testing and debugging the issue, I believe that HFS+ logic is correct.
> > > > 
> > > > As far as I can see, the generic/073 is checking specific btrfs related case:
> > > > 
> > > > # Test file A fsync after moving one other unrelated file B between directories
> > > > # and fsyncing B's old parent directory before fsyncing the file A. Check that
> > > > # after a crash all the file A data we fsynced is available.
> > > > #
> > > > # This test is motivated by an issue discovered in btrfs which caused the file
> > > > # data to be lost (despite fsync returning success to user space). That btrfs
> > > > # bug was fixed by the following linux kernel patch:
> > > > #
> > > > #   Btrfs: fix data loss in the fast fsync path
> > > > 
> > > > The test is doing these steps on final phase:
> > > > 
> > > > mv $SCRATCH_MNT/testdir_1/bar $SCRATCH_MNT/testdir_2/bar
> > > > $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir_1
> > > > $XFS_IO_PROG -c "fsync" $SCRATCH_MNT/foo
> > > > 
> > > > So, we move file bar from testdir_1 into testdir_2 folder. It means that HFS+
> > > > logic decrements the number of entries in testdir_1 and increments number of
> > > > entries in testdir_2. Finally, we do fsync only for testdir_1 and foo but not
> > > > for testdir_2.
> > > 
> > > If the fs is using journal, shouldn't the increments on the testdir_2
> > > already be journaled? Thus after a power loss, the increments on
> > > testdir_2/bar should be replayed thus the end user should still see that
> > > inode.
> > > 
> > 
> > Technically speaking, HFS+ is journaling file system in Apple implementation.
> > But we don't have this functionality implemented and fully supported on Linux
> > kernel side. Potentially, it can be done but currently we haven't such
> > functionality yet. So, HFS/HFS+ doesn't use journaling on Linux kernel side  and
> > no journal replay could happen. :)
> 
> That's fine, btrfs doesn't support (traditional) journal either, since 
> its metadata is already fully COW protected.
> 
> The journal-like part is called log-tree, which is only utilized for 
> speeding up fsync() so no full fs sync is needed (which is super 
> expensive for btrfs)
> 
> That's the reason why btrfs' fsync() is super complex and Filipe spent 
> tons of his time on this field, and it is btrfs that exposed this problem.
> 
> 
> But if HFS+ on linux doesn't support metadata journal, I'm afraid you 
> will need to go the hard path just like btrfs, to manually check if an 
> inode needs its parent inodes updated during fsync.

Yeah, you are right, this is one of the possible way. Let me check the
complexity of this change.

> > 
> > > To me this looks like a bug in HFS logic where something is not properly
> > > journaled (the increment on the testdir_2/bar).
> > > 
> > > 
> > 
> > This statement could be correct if we will support journaling for HFS+. But HFS
> > never supported the journaling technology.
> > 
> > > Finally, if you're asking an end user that if it is acceptable that
> > > after moving an inode and fsync the old directory, the inode is no
> > > longer reachable, I'm pretty sure no end user will think it's acceptable.
> > > 
> > 
> > As far as I can see, the fsck only corrects the number of entries for testdir_2
> > folder. The rest is pretty OK.
> 
> So it means the nlink number on the child inode is correct. I guess 
> since testdir_2 is not the directory where fsync is called, thus its 
> number of references is not updated.
> 
> With journal it will be much easier, but without it, you have to iterate 
> through the changed inode backrefs, and update the involved directories 
> one by one during fsync(), just like btrfs...
> 
> > 
> > > > As a result, this is the reason why fsck.hfsplus detects the
> > > > volume corruption afterwards. As far as I can see, the HFS+ driver behavior is
> > > > completely correct and nothing needs to be done for fixing in HFS+ logic here.
> > > 
> > > Then I guess you may also want to ask why EXT4/XFS/Btrfs/F2fs all pass
> > > the test case.
> > > 
> > 
> > So, ext4 and xfs support journaling. Maybe, it's interesting to check what is
> > going on with F2FS, FAT, and other file system that hasn't journaling support.
> 
> IIRC F2FS also support journals.
> 
> For VFAT the test case is rejected due to the lack of journal support.
> 
> On the other hand, if HFS+ doesn't support journal at all, you may want 
> to add hfs/hfs+ to _has_metadata_journaling(), so that the test case 
> will be skipped.
> 
> But still, since this test case already leads to fsck error reports, it 
> is worthy some improvement to avoid such problem.
> 

Make sense. Let me check the complexity of HFS+ modification to support
correctly such situation. But HFS should be added to _has_metadata_journaling()
if it is not already there.

Thanks,
Slava.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Why generic/073 is generic but not btrfs specific?
  2025-11-06 22:29       ` Viacheslav Dubeyko
@ 2025-11-08 14:01         ` Theodore Ts'o
  2025-11-10 19:41           ` Viacheslav Dubeyko
  0 siblings, 1 reply; 9+ messages in thread
From: Theodore Ts'o @ 2025-11-08 14:01 UTC (permalink / raw)
  To: Viacheslav Dubeyko
  Cc: fstests@vger.kernel.org, wqu@suse.com,
	linux-btrfs@vger.kernel.org, glaubitz@physik.fu-berlin.de,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org

On Thu, Nov 06, 2025 at 10:29:46PM +0000, Viacheslav Dubeyko wrote:
> > > Technically speaking, HFS+ is journaling file system in Apple implementation.
> > > But we don't have this functionality implemented and fully supported on Linux
> > > kernel side. Potentially, it can be done but currently we haven't such
> > > functionality yet. So, HFS/HFS+ doesn't use journaling on Linux kernel side  and
> > > no journal replay could happen. :)

If the implementation of HJFJS+ in Linux doesn't support metadata
consistency after a crash, I'd suggest adding HFS+ to
_has_metadat_journalling().  This will suppress a number of test
failures so you can focus on other issues which arguably is probably
higher priority for you to fix.

After you get HFS+ to run clean with the journalling tesets skipped,
then you can focus on adding that guarantee at that point, perhaps?

     	     	      	     	  - Ted

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC] Why generic/073 is generic but not btrfs specific?
  2025-11-08 14:01         ` Theodore Ts'o
@ 2025-11-10 19:41           ` Viacheslav Dubeyko
  2025-11-11  1:53             ` Zorro Lang
  0 siblings, 1 reply; 9+ messages in thread
From: Viacheslav Dubeyko @ 2025-11-10 19:41 UTC (permalink / raw)
  To: tytso@mit.edu
  Cc: fstests@vger.kernel.org, wqu@suse.com,
	glaubitz@physik.fu-berlin.de, linux-btrfs@vger.kernel.org,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org

On Sat, 2025-11-08 at 09:01 -0500, Theodore Ts'o wrote:
> On Thu, Nov 06, 2025 at 10:29:46PM +0000, Viacheslav Dubeyko wrote:
> > > > Technically speaking, HFS+ is journaling file system in Apple implementation.
> > > > But we don't have this functionality implemented and fully supported on Linux
> > > > kernel side. Potentially, it can be done but currently we haven't such
> > > > functionality yet. So, HFS/HFS+ doesn't use journaling on Linux kernel side  and
> > > > no journal replay could happen. :)
> 
> If the implementation of HJFJS+ in Linux doesn't support metadata
> consistency after a crash, I'd suggest adding HFS+ to
> _has_metadat_journalling().  This will suppress a number of test
> failures so you can focus on other issues which arguably is probably
> higher priority for you to fix.
> 
> After you get HFS+ to run clean with the journalling tesets skipped,
> then you can focus on adding that guarantee at that point, perhaps?
> 
> 

Yes, it makes sense. It's really good strategy. But I've decided to spend couple
of days on the fix of this issue. If I am not lucky to find the quick fix, then
I'll follow this strategy. :)

Thanks,
Slava.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Why generic/073 is generic but not btrfs specific?
  2025-11-10 19:41           ` Viacheslav Dubeyko
@ 2025-11-11  1:53             ` Zorro Lang
  2025-11-11  2:02               ` Viacheslav Dubeyko
  0 siblings, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2025-11-11  1:53 UTC (permalink / raw)
  To: Viacheslav Dubeyko
  Cc: tytso@mit.edu, fstests@vger.kernel.org, wqu@suse.com,
	glaubitz@physik.fu-berlin.de, linux-btrfs@vger.kernel.org,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org

On Mon, Nov 10, 2025 at 07:41:40PM +0000, Viacheslav Dubeyko wrote:
> On Sat, 2025-11-08 at 09:01 -0500, Theodore Ts'o wrote:
> > On Thu, Nov 06, 2025 at 10:29:46PM +0000, Viacheslav Dubeyko wrote:
> > > > > Technically speaking, HFS+ is journaling file system in Apple implementation.
> > > > > But we don't have this functionality implemented and fully supported on Linux
> > > > > kernel side. Potentially, it can be done but currently we haven't such
> > > > > functionality yet. So, HFS/HFS+ doesn't use journaling on Linux kernel side  and
> > > > > no journal replay could happen. :)
> > 
> > If the implementation of HJFJS+ in Linux doesn't support metadata
> > consistency after a crash, I'd suggest adding HFS+ to
> > _has_metadat_journalling().  This will suppress a number of test
> > failures so you can focus on other issues which arguably is probably
> > higher priority for you to fix.
> > 
> > After you get HFS+ to run clean with the journalling tesets skipped,
> > then you can focus on adding that guarantee at that point, perhaps?
> > 
> > 
> 
> Yes, it makes sense. It's really good strategy. But I've decided to spend couple
> of days on the fix of this issue. If I am not lucky to find the quick fix, then
> I'll follow this strategy. :)

Hi Slava,

fstests doesn't have an offical HFS+ supporting report (refer to README), so if you
find some helpers/cases can't work on hfs/hfsplus well, please feel free to modify
them to support hfs/hfsplus, then add hfs/hfsplus to the supporting list (in README)
after we make sure it works :)

Thanks,
Zorro

> 
> Thanks,
> Slava.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC] Why generic/073 is generic but not btrfs specific?
  2025-11-11  1:53             ` Zorro Lang
@ 2025-11-11  2:02               ` Viacheslav Dubeyko
  0 siblings, 0 replies; 9+ messages in thread
From: Viacheslav Dubeyko @ 2025-11-11  2:02 UTC (permalink / raw)
  To: Zirong Lang
  Cc: fstests@vger.kernel.org, wqu@suse.com,
	glaubitz@physik.fu-berlin.de, linux-btrfs@vger.kernel.org,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org, tytso@mit.edu

On Tue, 2025-11-11 at 09:53 +0800, Zorro Lang wrote:
> On Mon, Nov 10, 2025 at 07:41:40PM +0000, Viacheslav Dubeyko wrote:
> > On Sat, 2025-11-08 at 09:01 -0500, Theodore Ts'o wrote:
> > > On Thu, Nov 06, 2025 at 10:29:46PM +0000, Viacheslav Dubeyko wrote:
> > > > > > Technically speaking, HFS+ is journaling file system in Apple implementation.
> > > > > > But we don't have this functionality implemented and fully supported on Linux
> > > > > > kernel side. Potentially, it can be done but currently we haven't such
> > > > > > functionality yet. So, HFS/HFS+ doesn't use journaling on Linux kernel side  and
> > > > > > no journal replay could happen. :)
> > > 
> > > If the implementation of HJFJS+ in Linux doesn't support metadata
> > > consistency after a crash, I'd suggest adding HFS+ to
> > > _has_metadat_journalling().  This will suppress a number of test
> > > failures so you can focus on other issues which arguably is probably
> > > higher priority for you to fix.
> > > 
> > > After you get HFS+ to run clean with the journalling tesets skipped,
> > > then you can focus on adding that guarantee at that point, perhaps?
> > > 
> > > 
> > 
> > Yes, it makes sense. It's really good strategy. But I've decided to spend couple
> > of days on the fix of this issue. If I am not lucky to find the quick fix, then
> > I'll follow this strategy. :)
> 
> Hi Slava,
> 
> fstests doesn't have an offical HFS+ supporting report (refer to README), so if you
> find some helpers/cases can't work on hfs/hfsplus well, please feel free to modify
> them to support hfs/hfsplus, then add hfs/hfsplus to the supporting list (in README)
> after we make sure it works :)
> 

Hi Zorro,

Sounds good! I am working on it. Currently, I am trying to identify such test-
cases. I have several in mind but, currently, I am focused on test-cases that
require the fixes on HFS/HFS+ side.

I'll share the patches when I'll completely sure that HFS/HFS+ cannot manage
some xfstests cases.

Thanks,
Slava.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-11  2:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-06 20:40 [RFC] Why generic/073 is generic but not btrfs specific? Viacheslav Dubeyko
2025-11-06 20:58 ` Qu Wenruo
2025-11-06 21:11   ` Viacheslav Dubeyko
2025-11-06 21:32     ` Qu Wenruo
2025-11-06 22:29       ` Viacheslav Dubeyko
2025-11-08 14:01         ` Theodore Ts'o
2025-11-10 19:41           ` Viacheslav Dubeyko
2025-11-11  1:53             ` Zorro Lang
2025-11-11  2:02               ` Viacheslav Dubeyko

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).