[PATCH 1/2] hfsplus: fix to update ctime after rename

[PATCH 1/2] hfsplus: fix to update ctime after rename

[Yangtao Li]

[BUG]
$ sudo ./check generic/003
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 graphic 6.8.0-58-generic #60~22.04.1-Ubuntu
MKFS_OPTIONS  -- /dev/loop29
MOUNT_OPTIONS -- /dev/loop29 /mnt/scratch

generic/003       - output mismatch
    --- tests/generic/003.out   2025-04-27 08:49:39.876945323 -0600
    +++ /home/graphic/fs/xfstests-dev/results//generic/003.out.bad

     QA output created by 003
    +ERROR: change time has not been updated after changing file1
     Silence is golden
    ...

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

[CAUSE]
change time has not been updated after changing file1

[FIX]
Update file ctime after rename in hfsplus_rename().

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/hfsplus/dir.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 876bbb80fb4d..e77942440240 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -534,6 +534,7 @@ static int hfsplus_rename(struct mnt_idmap *idmap,
 			  struct inode *new_dir, struct dentry *new_dentry,
 			  unsigned int flags)
 {
+	struct inode *inode = d_inode(old_dentry);
 	int res;
 
 	if (flags & ~RENAME_NOREPLACE)
@@ -552,9 +553,13 @@ static int hfsplus_rename(struct mnt_idmap *idmap,
 	res = hfsplus_rename_cat((u32)(unsigned long)old_dentry->d_fsdata,
 				 old_dir, &old_dentry->d_name,
 				 new_dir, &new_dentry->d_name);
-	if (!res)
-		new_dentry->d_fsdata = old_dentry->d_fsdata;
-	return res;
+	if (res)
+		return res;
+
+	new_dentry->d_fsdata = old_dentry->d_fsdata;
+	inode_set_ctime_current(inode);
+	mark_inode_dirty(inode);
+	return 0;
 }
 
 const struct inode_operations hfsplus_dir_inode_operations = {
-- 
2.39.0

[PATCH 2/2] hfs: fix to update ctime after rename

[Yangtao Li]

Similar to hfsplus, let's update file ctime after the rename operation
in hfs_rename().

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/hfs/dir.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
index 86a6b317b474..3b95bafb3f04 100644
--- a/fs/hfs/dir.c
+++ b/fs/hfs/dir.c
@@ -284,6 +284,7 @@ static int hfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 		      struct dentry *old_dentry, struct inode *new_dir,
 		      struct dentry *new_dentry, unsigned int flags)
 {
+	struct inode *inode = d_inode(old_dentry);
 	int res;
 
 	if (flags & ~RENAME_NOREPLACE)
@@ -299,11 +300,15 @@ static int hfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 	res = hfs_cat_move(d_inode(old_dentry)->i_ino,
 			   old_dir, &old_dentry->d_name,
 			   new_dir, &new_dentry->d_name);
-	if (!res)
-		hfs_cat_build_key(old_dir->i_sb,
-				  (btree_key *)&HFS_I(d_inode(old_dentry))->cat_key,
-				  new_dir->i_ino, &new_dentry->d_name);
-	return res;
+	if (res)
+		return res;
+
+	hfs_cat_build_key(old_dir->i_sb,
+			  (btree_key *)&HFS_I(d_inode(old_dentry))->cat_key,
+			  new_dir->i_ino, &new_dentry->d_name);
+	inode_set_ctime_current(inode);
+	mark_inode_dirty(inode);
+	return 0;
 }
 
 const struct file_operations hfs_dir_operations = {
-- 
2.39.0

Re: [PATCH 2/2] hfs: fix to update ctime after rename

[Viacheslav Dubeyko]

On Tue, 2025-04-29 at 14:15 -0600, Yangtao Li wrote:
> Similar to hfsplus, let's update file ctime after the rename operation
> in hfs_rename().
> 

It will be good to see which HFS test-case fails in the comment.

> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  fs/hfs/dir.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
> index 86a6b317b474..3b95bafb3f04 100644
> --- a/fs/hfs/dir.c
> +++ b/fs/hfs/dir.c
> @@ -284,6 +284,7 @@ static int hfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  		      struct dentry *old_dentry, struct inode *new_dir,
>  		      struct dentry *new_dentry, unsigned int flags)
>  {
> +	struct inode *inode = d_inode(old_dentry);
>  	int res;
>  
>  	if (flags & ~RENAME_NOREPLACE)
> @@ -299,11 +300,15 @@ static int hfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  	res = hfs_cat_move(d_inode(old_dentry)->i_ino,
>  			   old_dir, &old_dentry->d_name,
>  			   new_dir, &new_dentry->d_name);
> -	if (!res)
> -		hfs_cat_build_key(old_dir->i_sb,
> -				  (btree_key *)&HFS_I(d_inode(old_dentry))->cat_key,
> -				  new_dir->i_ino, &new_dentry->d_name);
> -	return res;
> +	if (res)
> +		return res;
> +
> +	hfs_cat_build_key(old_dir->i_sb,
> +			  (btree_key *)&HFS_I(d_inode(old_dentry))->cat_key,
> +			  new_dir->i_ino, &new_dentry->d_name);
> +	inode_set_ctime_current(inode);
> +	mark_inode_dirty(inode);
> +	return 0;
>  }
> 

Unfortunately, I cannot apply this patch too. Could you please prepare the patch
for the latest kernel tree state?

Thanks,
Slava.

Re: [PATCH 2/2] hfs: fix to update ctime after rename

[Viacheslav Dubeyko]

On Tue, 2025-04-29 at 14:15 -0600, Yangtao Li wrote:
> Similar to hfsplus, let's update file ctime after the rename operation
> in hfs_rename().
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  fs/hfs/dir.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
> index 86a6b317b474..3b95bafb3f04 100644
> --- a/fs/hfs/dir.c
> +++ b/fs/hfs/dir.c
> @@ -284,6 +284,7 @@ static int hfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  		      struct dentry *old_dentry, struct inode *new_dir,
>  		      struct dentry *new_dentry, unsigned int flags)
>  {
> +	struct inode *inode = d_inode(old_dentry);
>  	int res;
>  
>  	if (flags & ~RENAME_NOREPLACE)
> @@ -299,11 +300,15 @@ static int hfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  	res = hfs_cat_move(d_inode(old_dentry)->i_ino,
>  			   old_dir, &old_dentry->d_name,
>  			   new_dir, &new_dentry->d_name);
> -	if (!res)
> -		hfs_cat_build_key(old_dir->i_sb,
> -				  (btree_key *)&HFS_I(d_inode(old_dentry))->cat_key,
> -				  new_dir->i_ino, &new_dentry->d_name);
> -	return res;
> +	if (res)
> +		return res;
> +
> +	hfs_cat_build_key(old_dir->i_sb,
> +			  (btree_key *)&HFS_I(d_inode(old_dentry))->cat_key,
> +			  new_dir->i_ino, &new_dentry->d_name);
> +	inode_set_ctime_current(inode);
> +	mark_inode_dirty(inode);
> +	return 0;
>  }
>  
>  const struct file_operations hfs_dir_operations = {

BEFORE PATCH:

uname -a
Linux hfsplus-testing-0001 6.15.0-rc4 #7 SMP PREEMPT_DYNAMIC Thu May  1 16:11:49
PDT 2025 x86_64 x86_64 x86_64 GNU/Linux

sudo ./check generic/003
FSTYP         -- hfs
PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4 #7 SMP
PREEMPT_DYNAMIC Thu May  1 16:11:49 PDT 2025
MKFS_OPTIONS  -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

generic/003       - output mismatch (see /home/slavad/XFSTESTS-2/xfstests-
dev/results//generic/003.out.bad)
    --- tests/generic/003.out	2025-04-24 12:48:45.886164335 -0700
    +++ /home/slavad/XFSTESTS-2/xfstests-
dev/results//generic/003.out.bad	2025-05-01 16:36:38.608591317 -0700
    @@ -1,2 +1,6 @@
     QA output created by 003
    +ERROR: access time has changed for file1 after remount
    +ERROR: access time has changed after modifying file1
    +ERROR: change time has not been updated after changing file1
    +ERROR: access time has changed for file in read-only filesystem
     Silence is golden
    ...
    (Run 'diff -u /home/slavad/XFSTESTS-2/xfstests-dev/tests/generic/003.out
/home/slavad/XFSTESTS-2/xfstests-dev/results//generic/003.out.bad'  to see the
entire diff)
Ran: generic/003
Failures: generic/003
Failed 1 of 1 tests

WITH APPLIED PATCH:

uname -a
Linux hfsplus-testing-0001 6.15.0-rc4+ #8 SMP PREEMPT_DYNAMIC Thu May  1
16:43:22 PDT 2025 x86_64 x86_64 x86_64 GNU/Linux

sudo ./check generic/003
FSTYP         -- hfs
PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP
PREEMPT_DYNAMIC Thu May  1 16:43:22 PDT 2025
MKFS_OPTIONS  -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

generic/003       - output mismatch (see /home/slavad/XFSTESTS-2/xfstests-
dev/results//generic/003.out.bad)
    --- tests/generic/003.out	2025-04-24 12:48:45.886164335 -0700
    +++ /home/slavad/XFSTESTS-2/xfstests-
dev/results//generic/003.out.bad	2025-05-01 16:53:56.628734257 -0700
    @@ -1,2 +1,5 @@
     QA output created by 003
    +ERROR: access time has changed for file1 after remount
    +ERROR: access time has changed after modifying file1
    +ERROR: access time has changed for file in read-only filesystem
     Silence is golden
    ...
    (Run 'diff -u /home/slavad/XFSTESTS-2/xfstests-dev/tests/generic/003.out
/home/slavad/XFSTESTS-2/xfstests-dev/results//generic/003.out.bad'  to see the
entire diff)
Ran: generic/003
Failures: generic/003
Failed 1 of 1 tests

It looks like that it is not the whole fix of the issue for HFS case.

Thanks,
Slava.

回复: [PATCH 2/2] hfs: fix to update ctime after rename

[李扬韬]

Hi Slava,

>    +ERROR: access time has changed for file1 after remount
>    +ERROR: access time has changed after modifying file1
>    +ERROR: access time has changed for file in read-only filesystem

>It looks like that it is not the whole fix of the issue for HFS case.

The test cases that failed after applying this patch are all related to the atime not being updated,
but hfs actually does not have atime. 

So the current fix is ​​sufficient, should we modify the 003 test case?

   dirCrDat:      LongInt;    {date and time of creation}
   dirMdDat:      LongInt;    {date and time of last modification}
   dirBkDat:      LongInt;    {date and time of last backup}

   filCrDat:      LongInt;    {date and time of creation}
   filMdDat:      LongInt;    {date and time of last modification}
   filBkDat:      LongInt;    {date and time of last backup}

Thanks,
Yangtao

Re: 回复: [PATCH 2/2] hfs: fix to update ctime after rename

[Viacheslav Dubeyko]

On Wed, 2025-05-07 at 14:22 +0000, 李扬韬 wrote:
> Hi Slava,
> 
> >    +ERROR: access time has changed for file1 after remount
> >    +ERROR: access time has changed after modifying file1
> >    +ERROR: access time has changed for file in read-only filesystem
> 
> > It looks like that it is not the whole fix of the issue for HFS
> > case.
> 
> The test cases that failed after applying this patch are all related
> to the atime not being updated,

If I understood correctly "ERROR: access time has changed for file1
after remount" means atime has been changed.

> but hfs actually does not have atime. 
> 

But how the test detects that atime has been updated? If HFS hasn't
atime, then test cannot detect such update, from my point of view.

> So the current fix is ​​sufficient, should we modify the 003 test
> case?
> 

I don't think so. Probably, something is wrong in HFS code. We need to
double check it.

Thanks,
Slava.

>    dirCrDat:      LongInt;    {date and time of creation}
>    dirMdDat:      LongInt;    {date and time of last modification}
>    dirBkDat:      LongInt;    {date and time of last backup}
> 
>    filCrDat:      LongInt;    {date and time of creation}
>    filMdDat:      LongInt;    {date and time of last modification}
>    filBkDat:      LongInt;    {date and time of last backup}
> 
> Thanks,
> Yangtao

Re: 回复: [PATCH 2/2] hfs: fix to update ctime after rename

[Viacheslav Dubeyko]

On Fri, 2025-05-09 at 10:51 -0700, Viacheslav Dubeyko wrote:
> On Wed, 2025-05-07 at 14:22 +0000, 李扬韬 wrote:
> > Hi Slava,
> > 
> > >    +ERROR: access time has changed for file1 after remount
> > >    +ERROR: access time has changed after modifying file1
> > >    +ERROR: access time has changed for file in read-only
> > > filesystem
> > 
> > > It looks like that it is not the whole fix of the issue for HFS
> > > case.
> > 
> > The test cases that failed after applying this patch are all
> > related
> > to the atime not being updated,
> 
> If I understood correctly "ERROR: access time has changed for file1
> after remount" means atime has been changed.
> 
> > but hfs actually does not have atime. 
> > 
> 
> But how the test detects that atime has been updated? If HFS hasn't
> atime, then test cannot detect such update, from my point of view.
> 

As far as I can see, HFS code operates by atime [1 - 3]. As a result,
generic_fillattr [4] can retrieve the atime value. If we don't store
atime in the on-disk metadata, then the test could see inconsistent
atime value. I believe that dirMdDat/filMdDat (mtime) needs to be
considered as atime too. Because, modification cannot be done without
access action. So, atime == mtime.

Thanks,
Slava.

> > So the current fix is ​​sufficient, should we modify the 003 test
> > case?
> > 
> 
> I don't think so. Probably, something is wrong in HFS code. We need
> to
> double check it.
> 
> Thanks,
> Slava.
> 
> >    dirCrDat:      LongInt;    {date and time of creation}
> >    dirMdDat:      LongInt;    {date and time of last modification}
> >    dirBkDat:      LongInt;    {date and time of last backup}
> > 
> >    filCrDat:      LongInt;    {date and time of creation}
> >    filMdDat:      LongInt;    {date and time of last modification}
> >    filBkDat:      LongInt;    {date and time of last backup}
> > 
> > Thanks,
> > Yangtao

[1] https://elixir.bootlin.com/linux/v6.12.6/source/fs/hfs/sysdep.c#L35
[2] https://elixir.bootlin.com/linux/v6.12.6/source/fs/hfs/inode.c#L356
[3] https://elixir.bootlin.com/linux/v6.12.6/source/fs/hfs/inode.c#L367
[4] https://elixir.bootlin.com/linux/v6.12.6/source/fs/stat.c#L60

回复: 回复: [PATCH 2/2] hfs: fix to update ctime after rename

[李扬韬]

Hi Slava,

> If I understood correctly "ERROR: access time has changed for file1 after remount" means atime has been changed.

In fact, it seems that it is not the atime that has been changed, but the disk atime that has been not changed. 
The inode in memory has a newer atime, but the atime is not updated to the disk when write_inode is executed(hfs has no atime in disk format).

For ERROR: access time has changed for file1 after remount

Before:
	Access:  2025-05-09 14:05:40
	Modify:  2025-05-09 14:05:38
	Change:  2025-05-09 14:05:38

After umount&mount:
	Access:  2025-05-09 14:05:38		<-- back to mtime
	Modify:  2025-05-09 14:05:38
	Change:  2025-05-09 14:05:38

So we get inconsistent results for atime.

Am I missing something?

Thx,
Yangtao

Re: 回复: 回复: [PATCH 2/2] hfs: fix to update ctime after rename

[Viacheslav Dubeyko]

On Sat, 2025-05-10 at 05:31 +0000, 李扬韬 wrote:
> Hi Slava,
> 
> > If I understood correctly "ERROR: access time has changed for file1
> > after remount" means atime has been changed.
> 
> In fact, it seems that it is not the atime that has been changed, but
> the disk atime that has been not changed. 
> The inode in memory has a newer atime, but the atime is not updated
> to the disk when write_inode is executed(hfs has no atime in disk
> format).
> 
> For ERROR: access time has changed for file1 after remount
> 
> Before:
> 	Access:  2025-05-09 14:05:40
> 	Modify:  2025-05-09 14:05:38
> 	Change:  2025-05-09 14:05:38
> 
> After umount&mount:
> 	Access:  2025-05-09 14:05:38		<-- back to mtime
> 	Modify:  2025-05-09 14:05:38
> 	Change:  2025-05-09 14:05:38
> 
> So we get inconsistent results for atime.
> 
> Am I missing something?
> 

This is was my point. We need to make the access time (atime) always
the same as the modification time (mtime) because we cannot save the
atime on disk. This is what I meant by atime == mtime. And this atime
change is happening in HFS driver logic that needs to be corrected.

Thanks,
Slava.

Re: [PATCH 1/2] hfsplus: fix to update ctime after rename

[Viacheslav Dubeyko]

On Tue, 2025-04-29 at 14:15 -0600, Yangtao Li wrote:
> [BUG]
> $ sudo ./check generic/003
> FSTYP         -- hfsplus
> PLATFORM      -- Linux/x86_64 graphic 6.8.0-58-generic #60~22.04.1-Ubuntu
> MKFS_OPTIONS  -- /dev/loop29
> MOUNT_OPTIONS -- /dev/loop29 /mnt/scratch
> 
> generic/003       - output mismatch
>     --- tests/generic/003.out   2025-04-27 08:49:39.876945323 -0600
>     +++ /home/graphic/fs/xfstests-dev/results//generic/003.out.bad
> 
>      QA output created by 003
>     +ERROR: change time has not been updated after changing file1
>      Silence is golden
>     ...
> 
> Ran: generic/003
> Failures: generic/003
> Failed 1 of 1 tests
> 
> [CAUSE]
> change time has not been updated after changing file1
> 
> [FIX]
> Update file ctime after rename in hfsplus_rename().
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  fs/hfsplus/dir.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index 876bbb80fb4d..e77942440240 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -534,6 +534,7 @@ static int hfsplus_rename(struct mnt_idmap *idmap,
>  			  struct inode *new_dir, struct dentry *new_dentry,
>  			  unsigned int flags)
>  {

Unfortunately, I am unable to apply you patch. In 6.15-rc4 we have already:

static int hfsplus_rename(struct mnt_idmap *idmap,
			  struct inode *old_dir, struct dentry *old_dentry,
			  struct inode *new_dir, struct dentry *new_dentry,
			  unsigned int flags)

Could you please prepare the patch for latest state of the kernel tree?

Thanks,
Slava.

回复: [PATCH 1/2] hfsplus: fix to update ctime after rename

[李扬韬]

Hi Slava,

> Unfortunately, I am unable to apply you patch. In 6.15-rc4 we have already:

Why can't apply?

> Could you please prepare the patch for latest state of the kernel tree?

In fact, I applied these two patches to 6.15-rc4, and there were no abnormalities.
based on commit 4f79eaa2ceac86a0e0f304b0bab556cca5bf4f30

Thanks,
Yangtao

Re: 回复: [PATCH 1/2] hfsplus: fix to update ctime after rename

[Viacheslav Dubeyko]

Hi Yangtao,

On Thu, 2025-05-01 at 12:01 +0000, 李扬韬 wrote:
> Hi Slava,
> 
> > Unfortunately, I am unable to apply you patch. In 6.15-rc4 we have already:
> 
> Why can't apply?
> 
> > Could you please prepare the patch for latest state of the kernel tree?
> 
> In fact, I applied these two patches to 6.15-rc4, and there were no abnormalities.
> based on commit 4f79eaa2ceac86a0e0f304b0bab556cca5bf4f30
> 

It's strange. The 'git apply' doesn't work, but 'git am' does work.

git apply -v ./\[EXTERNAL\]\ \[PATCH\ 1_2\]\ hfsplus\:\ fix\ to\ update\ ctime\
after\ rename.mbox
Checking patch fs/hfsplus/dir.c...
error: while searching for:
			  struct inode *new_dir, struct dentry *new_dentry,?
			  unsigned int flags)?
{?
	int res;?
?
	if (flags & ~RENAME_NOREPLACE)?

error: patch failed: fs/hfsplus/dir.c:534
error: fs/hfsplus/dir.c: patch does not apply

git am ./\[EXTERNAL\]\ \[PATCH\ 1_2\]\ hfsplus\:\ fix\ to\ update\ ctime\ after\
rename.mbox
Applying: hfsplus: fix to update ctime after rename

Does 'git apply' works on your side if you try to apply the patch from email? Is
it some glitch on my side? As far as I can see, I am trying to apply on clean
kernel tree.

Thanks,
Slava.

Re: 回复: [PATCH 1/2] hfsplus: fix to update ctime after rename

[John Paul Adrian Glaubitz]

Hi Slava,

On Thu, 2025-05-01 at 18:57 +0000, Viacheslav Dubeyko wrote:
> On Thu, 2025-05-01 at 12:01 +0000, 李扬韬 wrote:
> > Hi Slava,
> > 
> > > Unfortunately, I am unable to apply you patch. In 6.15-rc4 we have already:
> > 
> > Why can't apply?
> > 
> > > Could you please prepare the patch for latest state of the kernel tree?
> > 
> > In fact, I applied these two patches to 6.15-rc4, and there were no abnormalities.
> > based on commit 4f79eaa2ceac86a0e0f304b0bab556cca5bf4f30
> > 
> 
> It's strange. The 'git apply' doesn't work, but 'git am' does work.
> 
> git apply -v ./\[EXTERNAL\]\ \[PATCH\ 1_2\]\ hfsplus\:\ fix\ to\ update\ ctime\
> after\ rename.mbox
> Checking patch fs/hfsplus/dir.c...
> error: while searching for:
> 			  struct inode *new_dir, struct dentry *new_dentry,?
> 			  unsigned int flags)?
> {?
> 	int res;?
> ?
> 	if (flags & ~RENAME_NOREPLACE)?
> 
> error: patch failed: fs/hfsplus/dir.c:534
> error: fs/hfsplus/dir.c: patch does not apply
> 
> git am ./\[EXTERNAL\]\ \[PATCH\ 1_2\]\ hfsplus\:\ fix\ to\ update\ ctime\ after\
> rename.mbox
> Applying: hfsplus: fix to update ctime after rename
> 
> Does 'git apply' works on your side if you try to apply the patch from email? Is
> it some glitch on my side? As far as I can see, I am trying to apply on clean
> kernel tree.

git-apply is for applying plain patches while git-am is for patches from mailboxes.

I would suggest to always use git-am after fetching patches using "b4 am <MSGID>"
from the kernel mailing list.

Please note that when you apply patches with git-am, you should always use the "-s"
option so that the patches are automatically signed-off with your own email address.

Btw, can you push your tree somewhere until you've got your kernel.org account?

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

RE: 回复: [PATCH 1/2] hfsplus: fix to update ctime after rename

[Viacheslav Dubeyko]

On Thu, 2025-05-01 at 21:10 +0200, John Paul Adrian Glaubitz wrote:
> Hi Slava,
> 
> 

<skipped>

> > 
> > Does 'git apply' works on your side if you try to apply the patch from email? Is
> > it some glitch on my side? As far as I can see, I am trying to apply on clean
> > kernel tree.
> 
> git-apply is for applying plain patches while git-am is for patches from mailboxes.
> 
> I would suggest to always use git-am after fetching patches using "b4 am <MSGID>"
> from the kernel mailing list.
> 
> Please note that when you apply patches with git-am, you should always use the "-s"
> option so that the patches are automatically signed-off with your own email address.
> 

Sorry, my glitch. :)

> Btw, can you push your tree somewhere until you've got your kernel.org account?
> 

Do we really need to create some temporary tree? I have a fork of kernel tree on
github where I am managing SSDFS source code. But I am not sure that I can
create another fork of kernel tree on github.

Thanks,
Slava.

Re: 回复: [PATCH 1/2] hfsplus: fix to update ctime after rename

[Theodore Ts'o]

On Thu, May 01, 2025 at 07:48:35PM +0000, Viacheslav Dubeyko wrote:
> > Please note that when you apply patches with git-am, you should
> > always use the "-s" option so that the patches are automatically
> > signed-off with your own email address.

Pro tip: If you are using "b4 am" to download patches from
lore.kernel.org, use the -c option, e.g., "b4 am -c [msgid]".  This
will automatically check to make sure the patches have valid DKIM
headers, etc. and will also check to see if there is a newer version
of the patch series on lore.kernel.org and download it instead.

Another cool command is "b4 shazaam"; see the b4 man page for more details.

> > Btw, can you push your tree somewhere until you've got your
> > kernel.org account?
> 
> Do we really need to create some temporary tree? I have a fork of
> kernel tree on github where I am managing SSDFS source code. But I
> am not sure that I can create another fork of kernel tree on github.

If you have a fork of the kernel tree, sure, you can just use that and
tell folks what branch they should look at.

Github should be just fine creating another fork of the kernel tree,
however.  One advantage of having separate forks is that once you set
up the kernel.org tree from your local git tree, it becomes easier to
update multiple trees via separate git trees.  So for example, when I
push out changes, I might do

git push ra     # ra.kernel.org is a CNAME for gitolite.kernel.org
                # and is easier to type.   :-)
git push github

... and this will update my trees on kernel.org and github
automatically, aince I have in my .git/config file:

[remote "ra"]
	url = ssh://gitolite@ra.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git
	fetch = +refs/heads/*:refs/remotes/ext4/*
	push = +master:master
	push = +origin:origin
	push = +fixes:fixes
	push = +dev:dev
	push = +test:test

[remote "github"]
	url = git@github.com:tytso/ext4.git
	fetch = +refs/heads/*:refs/remotes/github/*
	push = +master:master
	push = +origin:origin
	push = +fixes:fixes
	push = +dev:dev
	push = +test:test

Cheers,

						- Ted

RE: 回复: [PATCH 1/2] hfsplus: fix to update ctime after rename

[Viacheslav Dubeyko]

On Fri, 2025-05-02 at 08:52 -0400, Theodore Ts'o wrote:
> On Thu, May 01, 2025 at 07:48:35PM +0000, Viacheslav Dubeyko wrote:
> > > 

<skipped>

> 
> > > Btw, can you push your tree somewhere until you've got your
> > > kernel.org account?
> > 
> > Do we really need to create some temporary tree? I have a fork of
> > kernel tree on github where I am managing SSDFS source code. But I
> > am not sure that I can create another fork of kernel tree on github.
> 
> If you have a fork of the kernel tree, sure, you can just use that and
> tell folks what branch they should look at.
> 
> Github should be just fine creating another fork of the kernel tree,
> however.  One advantage of having separate forks is that once you set
> up the kernel.org tree from your local git tree, it becomes easier to
> update multiple trees via separate git trees.  So for example, when I
> push out changes, I might do
> 
> git push ra     # ra.kernel.org is a CNAME for gitolite.kernel.org
>                 # and is easier to type.   :-)
> git push github
> 
> ... and this will update my trees on kernel.org and github
> automatically, aince I have in my .git/config file:
> 
> [remote "ra"]
> 	url = ssh://gitolite@ra.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git
> 	fetch = +refs/heads/*:refs/remotes/ext4/*
> 	push = +master:master
> 	push = +origin:origin
> 	push = +fixes:fixes
> 	push = +dev:dev
> 	push = +test:test
> 
> [remote "github"]
> 	url = git@github.com:tytso/ext4.git
> 	fetch = +refs/heads/*:refs/remotes/github/*
> 	push = +master:master
> 	push = +origin:origin
> 	push = +fixes:fixes
> 	push = +dev:dev
> 	push = +test:test
> 

Makes sense! Thank you for sharing these details. :)

Adrian, Yangtao,

I've created a hfs-linux-kernel organization [1] on GitHub and I've sent
invitation to both of you. I've made the kernel tree fork [2] and I've started
of collecting the known HFS/HFS+ issues [3]. So, we can take and assign the
issue(s) there. Also, we could have a WiKi there too [4].

So, we are preparing the environment step by step. :)

Thanks,
Slava.

[1] https://github.com/hfs-linux-kernel
[2] https://github.com/hfs-linux-kernel/hfs-linux-kernel
[3] https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues/
[4] https://github.com/hfs-linux-kernel/hfs-linux-kernel/wiki

Re: [PATCH 1/2] hfsplus: fix to update ctime after rename

[Viacheslav Dubeyko]

On Tue, 2025-04-29 at 14:15 -0600, Yangtao Li wrote:
> [BUG]
> $ sudo ./check generic/003
> FSTYP         -- hfsplus
> PLATFORM      -- Linux/x86_64 graphic 6.8.0-58-generic #60~22.04.1-Ubuntu
> MKFS_OPTIONS  -- /dev/loop29
> MOUNT_OPTIONS -- /dev/loop29 /mnt/scratch
> 
> generic/003       - output mismatch
>     --- tests/generic/003.out   2025-04-27 08:49:39.876945323 -0600
>     +++ /home/graphic/fs/xfstests-dev/results//generic/003.out.bad
> 
>      QA output created by 003
>     +ERROR: change time has not been updated after changing file1
>      Silence is golden
>     ...
> 
> Ran: generic/003
> Failures: generic/003
> Failed 1 of 1 tests
> 
> [CAUSE]
> change time has not been updated after changing file1
> 
> [FIX]
> Update file ctime after rename in hfsplus_rename().
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  fs/hfsplus/dir.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index 876bbb80fb4d..e77942440240 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -534,6 +534,7 @@ static int hfsplus_rename(struct mnt_idmap *idmap,
>  			  struct inode *new_dir, struct dentry *new_dentry,
>  			  unsigned int flags)
>  {
> +	struct inode *inode = d_inode(old_dentry);
>  	int res;
>  
>  	if (flags & ~RENAME_NOREPLACE)
> @@ -552,9 +553,13 @@ static int hfsplus_rename(struct mnt_idmap *idmap,
>  	res = hfsplus_rename_cat((u32)(unsigned long)old_dentry->d_fsdata,
>  				 old_dir, &old_dentry->d_name,
>  				 new_dir, &new_dentry->d_name);
> -	if (!res)
> -		new_dentry->d_fsdata = old_dentry->d_fsdata;
> -	return res;
> +	if (res)
> +		return res;
> +
> +	new_dentry->d_fsdata = old_dentry->d_fsdata;
> +	inode_set_ctime_current(inode);
> +	mark_inode_dirty(inode);
> +	return 0;
>  }
>  
>  const struct inode_operations hfsplus_dir_inode_operations = {

BEFORE PATCH:

sudo ./check generic/003
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4 #7 SMP
PREEMPT_DYNAMIC Thu May  1 16:11:49 PDT 2025
MKFS_OPTIONS  -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

generic/003       - output mismatch (see /home/slavad/XFSTESTS-2/xfstests-
dev/results//generic/003.out.bad)
    --- tests/generic/003.out	2025-04-24 12:48:45.886164335 -0700
    +++ /home/slavad/XFSTESTS-2/xfstests-
dev/results//generic/003.out.bad	2025-05-01 16:42:51.220196434 -0700
    @@ -1,2 +1,3 @@
     QA output created by 003
    +ERROR: change time has not been updated after changing file1
     Silence is golden
    ...
    (Run 'diff -u /home/slavad/XFSTESTS-2/xfstests-dev/tests/generic/003.out
/home/slavad/XFSTESTS-2/xfstests-dev/results//generic/003.out.bad'  to see the
entire diff)
Ran: generic/003
Failures: generic/003
Failed 1 of 1 tests

WITH APPLIED PATCH:

sudo ./check generic/003
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP
PREEMPT_DYNAMIC Thu May  1 16:43:22 PDT 2025
MKFS_OPTIONS  -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

generic/003        35s
Ran: generic/003
Passed all 1 tests

Tested-by: Viacheslav Dubeyko <slava@dubeyko.com>
Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.