[PATCH 0/2] fat: revise the FAT patches for mgtime series

[PATCH 0/2] fat: revise the FAT patches for mgtime series

[Jeff Layton]

This is a respin of just the FAT patches for the multigrain ctime
series. It's based on top of Christian's vfs.ctime branch, with this
patch reverted:

    89b39bea91c4 fat: make fat_update_time get its own timestamp

Christian, let me know if you'd rather I resend the whole series.

To: Christian Brauner <brauner@kernel.org>
To: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
To: Frank Sorenson <sorenson@redhat.com>
To: Jan Kara <jack@suse.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (2):
      fat: remove i_version handling from fat_update_time
      fat: make fat_update_time get its own timestamp

 fs/fat/misc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
---
base-commit: bae85436bd3cca48ba625d0caf7457fe7708c336
change-id: 20230810-ctime-fat-e4be14feb165

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH 1/2] fat: remove i_version handling from fat_update_time

[Jeff Layton]

commit 6bb885ecd746 (fat: add functions to update and truncate
timestamps appropriately") added an update_time routine for fat. That
patch added a section for handling the S_VERSION bit, even though FAT
doesn't enable SB_I_VERSION and the S_VERSION bit will never be set when
calling it.

Remove the section for handling S_VERSION since it's effectively dead
code, and will be problematic vs. future changes.

Cc: Frank Sorenson <sorenson@redhat.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/fat/misc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index ab28173348fa..37f4afb346af 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -354,9 +354,6 @@ int fat_update_time(struct inode *inode, int flags)
 			dirty_flags |= I_DIRTY_SYNC;
 	}
 
-	if ((flags & S_VERSION) && inode_maybe_inc_iversion(inode, false))
-		dirty_flags |= I_DIRTY_SYNC;
-
 	__mark_inode_dirty(inode, dirty_flags);
 	return 0;
 }

-- 
2.41.0

Re: [PATCH 1/2] fat: remove i_version handling from fat_update_time

[Jan Kara]

On Thu 10-08-23 09:12:04, Jeff Layton wrote:
> commit 6bb885ecd746 (fat: add functions to update and truncate
> timestamps appropriately") added an update_time routine for fat. That
> patch added a section for handling the S_VERSION bit, even though FAT
> doesn't enable SB_I_VERSION and the S_VERSION bit will never be set when
> calling it.
> 
> Remove the section for handling S_VERSION since it's effectively dead
> code, and will be problematic vs. future changes.
> 
> Cc: Frank Sorenson <sorenson@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/fat/misc.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index ab28173348fa..37f4afb346af 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -354,9 +354,6 @@ int fat_update_time(struct inode *inode, int flags)
>  			dirty_flags |= I_DIRTY_SYNC;
>  	}
>  
> -	if ((flags & S_VERSION) && inode_maybe_inc_iversion(inode, false))
> -		dirty_flags |= I_DIRTY_SYNC;
> -
>  	__mark_inode_dirty(inode, dirty_flags);
>  	return 0;
>  }
> 
> -- 
> 2.41.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/2] fat: remove i_version handling from fat_update_time

[OGAWA Hirofumi]

Jeff Layton <jlayton@kernel.org> writes:

> commit 6bb885ecd746 (fat: add functions to update and truncate
> timestamps appropriately") added an update_time routine for fat. That
> patch added a section for handling the S_VERSION bit, even though FAT
> doesn't enable SB_I_VERSION and the S_VERSION bit will never be set when
> calling it.
>
> Remove the section for handling S_VERSION since it's effectively dead
> code, and will be problematic vs. future changes.
>
> Cc: Frank Sorenson <sorenson@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Thanks.

> ---
>  fs/fat/misc.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index ab28173348fa..37f4afb346af 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -354,9 +354,6 @@ int fat_update_time(struct inode *inode, int flags)
>  			dirty_flags |= I_DIRTY_SYNC;
>  	}
>  
> -	if ((flags & S_VERSION) && inode_maybe_inc_iversion(inode, false))
> -		dirty_flags |= I_DIRTY_SYNC;
> -
>  	__mark_inode_dirty(inode, dirty_flags);
>  	return 0;
>  }

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH 1/2] fat: remove i_version handling from fat_update_time

[Frank Sorenson]

On 8/10/23 08:12, Jeff Layton wrote:
> commit 6bb885ecd746 (fat: add functions to update and truncate
> timestamps appropriately") added an update_time routine for fat. That
> patch added a section for handling the S_VERSION bit, even though FAT
> doesn't enable SB_I_VERSION and the S_VERSION bit will never be set when
> calling it.
>
> Remove the section for handling S_VERSION since it's effectively dead
> code, and will be problematic vs. future changes.
>
> Cc: Frank Sorenson <sorenson@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---

Reviewed-by: Frank Sorenson <sorenson@redhat.com>

>   fs/fat/misc.c | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index ab28173348fa..37f4afb346af 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -354,9 +354,6 @@ int fat_update_time(struct inode *inode, int flags)
>   			dirty_flags |= I_DIRTY_SYNC;
>   	}
>   
> -	if ((flags & S_VERSION) && inode_maybe_inc_iversion(inode, false))
> -		dirty_flags |= I_DIRTY_SYNC;
> -
>   	__mark_inode_dirty(inode, dirty_flags);
>   	return 0;
>   }
>

[PATCH 2/2] fat: make fat_update_time get its own timestamp

[Jeff Layton]

In later patches, we're going to drop the "now" parameter from the
update_time operation. Fix fat_update_time to fetch its own timestamp.
It turns out that this is easily done by just passing a NULL timestamp
pointer to fat_truncate_time.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/fat/misc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 37f4afb346af..f2304a1054aa 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -347,7 +347,7 @@ int fat_update_time(struct inode *inode, int flags)
 		return 0;
 
 	if (flags & (S_ATIME | S_CTIME | S_MTIME)) {
-		fat_truncate_time(inode, now, flags);
+		fat_truncate_time(inode, NULL, flags);
 		if (inode->i_sb->s_flags & SB_LAZYTIME)
 			dirty_flags |= I_DIRTY_TIME;
 		else

-- 
2.41.0

Re: [PATCH 2/2] fat: make fat_update_time get its own timestamp

[Jan Kara]

On Thu 10-08-23 09:12:05, Jeff Layton wrote:
> In later patches, we're going to drop the "now" parameter from the
> update_time operation. Fix fat_update_time to fetch its own timestamp.
> It turns out that this is easily done by just passing a NULL timestamp
> pointer to fat_truncate_time.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/fat/misc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index 37f4afb346af..f2304a1054aa 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -347,7 +347,7 @@ int fat_update_time(struct inode *inode, int flags)
>  		return 0;
>  
>  	if (flags & (S_ATIME | S_CTIME | S_MTIME)) {
> -		fat_truncate_time(inode, now, flags);
> +		fat_truncate_time(inode, NULL, flags);
>  		if (inode->i_sb->s_flags & SB_LAZYTIME)
>  			dirty_flags |= I_DIRTY_TIME;
>  		else
> 
> -- 
> 2.41.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/2] fat: make fat_update_time get its own timestamp

[OGAWA Hirofumi]

Jeff Layton <jlayton@kernel.org> writes:

> In later patches, we're going to drop the "now" parameter from the
> update_time operation. Fix fat_update_time to fetch its own timestamp.
> It turns out that this is easily done by just passing a NULL timestamp
> pointer to fat_truncate_time.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Thanks.

> ---
>  fs/fat/misc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index 37f4afb346af..f2304a1054aa 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -347,7 +347,7 @@ int fat_update_time(struct inode *inode, int flags)
>  		return 0;
>  
>  	if (flags & (S_ATIME | S_CTIME | S_MTIME)) {
> -		fat_truncate_time(inode, now, flags);
> +		fat_truncate_time(inode, NULL, flags);
>  		if (inode->i_sb->s_flags & SB_LAZYTIME)
>  			dirty_flags |= I_DIRTY_TIME;
>  		else

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH 2/2] fat: make fat_update_time get its own timestamp

[Frank Sorenson]

On 8/10/23 08:12, Jeff Layton wrote:
> In later patches, we're going to drop the "now" parameter from the
> update_time operation. Fix fat_update_time to fetch its own timestamp.
> It turns out that this is easily done by just passing a NULL timestamp
> pointer to fat_truncate_time.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---

Reviewed-by:  Frank Sorenson <sorenson@redhat.com>


>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index 37f4afb346af..f2304a1054aa 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -347,7 +347,7 @@ int fat_update_time(struct inode *inode, int flags)
>   		return 0;
>   
>   	if (flags & (S_ATIME | S_CTIME | S_MTIME)) {
> -		fat_truncate_time(inode, now, flags);
> +		fat_truncate_time(inode, NULL, flags);
>   		if (inode->i_sb->s_flags & SB_LAZYTIME)
>   			dirty_flags |= I_DIRTY_TIME;
>   		else
>

Re: [PATCH 0/2] fat: revise the FAT patches for mgtime series

[Christian Brauner]

On Thu, Aug 10, 2023 at 09:12:03AM -0400, Jeff Layton wrote:
> This is a respin of just the FAT patches for the multigrain ctime
> series. It's based on top of Christian's vfs.ctime branch, with this
> patch reverted:
> 
>     89b39bea91c4 fat: make fat_update_time get its own timestamp
> 
> Christian, let me know if you'd rather I resend the whole series.

Nono, that's fine as is. Thanks for taking care of all the fixups!

Re: [PATCH 0/2] fat: revise the FAT patches for mgtime series

[Christian Brauner]

On Thu, 10 Aug 2023 09:12:03 -0400, Jeff Layton wrote:
> This is a respin of just the FAT patches for the multigrain ctime
> series. It's based on top of Christian's vfs.ctime branch, with this
> patch reverted:
> 
>     89b39bea91c4 fat: make fat_update_time get its own timestamp
> 
> Christian, let me know if you'd rather I resend the whole series.
> 
> [...]

I simply dropped the old fat patch and applied these two instead. I've
rebased them so their in the same position as the old patch so the
series is still logically ordered.

---

Applied to the vfs.ctime branch of the vfs/vfs.git tree.
Patches in the vfs.ctime branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.ctime

[1/2] fat: remove i_version handling from fat_update_time
      https://git.kernel.org/vfs/vfs/c/93e6c3043544
[2/2] fat: make fat_update_time get its own timestamp
      https://git.kernel.org/vfs/vfs/c/6f4aaee3faa8