Re: [PATCH v2 62/92] ocfs2: convert to ctime accessor functions
From: Joseph Qi
Date: Sun Jul 09 2023 - 09:59:22 EST
On 7/7/23 6:07 PM, Jeff Layton wrote:
> On Fri, 2023-07-07 at 11:15 +0800, Joseph Qi wrote:
>>
>> On 7/6/23 3:01 AM, Jeff Layton wrote:
>>> In later patches, we're going to change how the inode's ctime field is
>>> used. Switch to using accessor functions instead of raw accesses of
>>> inode->i_ctime.
>>>
>>> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>>> ---
>>> fs/ocfs2/acl.c | 6 +++---
>>> fs/ocfs2/alloc.c | 6 +++---
>>> fs/ocfs2/aops.c | 2 +-
>>> fs/ocfs2/dir.c | 8 ++++----
>>> fs/ocfs2/dlmfs/dlmfs.c | 4 ++--
>>> fs/ocfs2/dlmglue.c | 7 +++++--
>>> fs/ocfs2/file.c | 16 +++++++++-------
>>> fs/ocfs2/inode.c | 12 ++++++------
>>> fs/ocfs2/move_extents.c | 6 +++---
>>> fs/ocfs2/namei.c | 21 +++++++++++----------
>>> fs/ocfs2/refcounttree.c | 14 +++++++-------
>>> fs/ocfs2/xattr.c | 6 +++---
>>> 12 files changed, 57 insertions(+), 51 deletions(-)
>>>
>>> diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c
>>> index 9fd03eaf15f8..e75137a8e7cb 100644
>>> --- a/fs/ocfs2/acl.c
>>> +++ b/fs/ocfs2/acl.c
>>> @@ -191,10 +191,10 @@ static int ocfs2_acl_set_mode(struct inode *inode, struct buffer_head *di_bh,
>>> }
>>>
>>> inode->i_mode = new_mode;
>>> - inode->i_ctime = current_time(inode);
>>> + inode_set_ctime_current(inode);
>>> di->i_mode = cpu_to_le16(inode->i_mode);
>>> - di->i_ctime = cpu_to_le64(inode->i_ctime.tv_sec);
>>> - di->i_ctime_nsec = cpu_to_le32(inode->i_ctime.tv_nsec);
>>> + di->i_ctime = cpu_to_le64(inode_get_ctime(inode).tv_sec);
>>> + di->i_ctime_nsec = cpu_to_le32(inode_get_ctime(inode).tv_nsec);
>>> ocfs2_update_inode_fsync_trans(handle, inode, 0);
>>>
>>> ocfs2_journal_dirty(handle, di_bh);
>>> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
>>> index 51c93929a146..aef58f1395c8 100644
>>> --- a/fs/ocfs2/alloc.c
>>> +++ b/fs/ocfs2/alloc.c
>>> @@ -7436,10 +7436,10 @@ int ocfs2_truncate_inline(struct inode *inode, struct buffer_head *di_bh,
>>> }
>>>
>>> inode->i_blocks = ocfs2_inode_sector_count(inode);
>>> - inode->i_ctime = inode->i_mtime = current_time(inode);
>>> + inode->i_mtime = inode_set_ctime_current(inode);
>>>
>>> - di->i_ctime = di->i_mtime = cpu_to_le64(inode->i_ctime.tv_sec);
>>> - di->i_ctime_nsec = di->i_mtime_nsec = cpu_to_le32(inode->i_ctime.tv_nsec);
>>> + di->i_ctime = di->i_mtime = cpu_to_le64(inode_get_ctime(inode).tv_sec);
>>> + di->i_ctime_nsec = di->i_mtime_nsec = cpu_to_le32(inode_get_ctime(inode).tv_nsec);
>>>
>>> ocfs2_update_inode_fsync_trans(handle, inode, 1);
>>> ocfs2_journal_dirty(handle, di_bh);
>>> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
>>> index 8dfc284e85f0..0fdba30740ab 100644
>>> --- a/fs/ocfs2/aops.c
>>> +++ b/fs/ocfs2/aops.c
>>> @@ -2048,7 +2048,7 @@ int ocfs2_write_end_nolock(struct address_space *mapping,
>>> }
>>> inode->i_blocks = ocfs2_inode_sector_count(inode);
>>> di->i_size = cpu_to_le64((u64)i_size_read(inode));
>>> - inode->i_mtime = inode->i_ctime = current_time(inode);
>>> + inode->i_mtime = inode_set_ctime_current(inode);
>>> di->i_mtime = di->i_ctime = cpu_to_le64(inode->i_mtime.tv_sec);
>>> di->i_mtime_nsec = di->i_ctime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec);
>>> if (handle)
>>> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
>>> index 694471fc46b8..8b123d543e6e 100644
>>> --- a/fs/ocfs2/dir.c
>>> +++ b/fs/ocfs2/dir.c
>>> @@ -1658,7 +1658,7 @@ int __ocfs2_add_entry(handle_t *handle,
>>> offset, ocfs2_dir_trailer_blk_off(dir->i_sb));
>>>
>>> if (ocfs2_dirent_would_fit(de, rec_len)) {
>>> - dir->i_mtime = dir->i_ctime = current_time(dir);
>>> + dir->i_mtime = inode_set_ctime_current(dir);
>>> retval = ocfs2_mark_inode_dirty(handle, dir, parent_fe_bh);
>>> if (retval < 0) {
>>> mlog_errno(retval);
>>> @@ -2962,11 +2962,11 @@ static int ocfs2_expand_inline_dir(struct inode *dir, struct buffer_head *di_bh,
>>> ocfs2_dinode_new_extent_list(dir, di);
>>>
>>> i_size_write(dir, sb->s_blocksize);
>>> - dir->i_mtime = dir->i_ctime = current_time(dir);
>>> + dir->i_mtime = inode_set_ctime_current(dir);
>>>
>>> di->i_size = cpu_to_le64(sb->s_blocksize);
>>> - di->i_ctime = di->i_mtime = cpu_to_le64(dir->i_ctime.tv_sec);
>>> - di->i_ctime_nsec = di->i_mtime_nsec = cpu_to_le32(dir->i_ctime.tv_nsec);
>>> + di->i_ctime = di->i_mtime = cpu_to_le64(inode_get_ctime(dir).tv_sec);
>>> + di->i_ctime_nsec = di->i_mtime_nsec = cpu_to_le32(inode_get_ctime(dir).tv_nsec);
>>> ocfs2_update_inode_fsync_trans(handle, dir, 1);
>>>
>>> /*
>>> diff --git a/fs/ocfs2/dlmfs/dlmfs.c b/fs/ocfs2/dlmfs/dlmfs.c
>>> index ba26c5567cff..81265123ce6c 100644
>>> --- a/fs/ocfs2/dlmfs/dlmfs.c
>>> +++ b/fs/ocfs2/dlmfs/dlmfs.c
>>> @@ -337,7 +337,7 @@ static struct inode *dlmfs_get_root_inode(struct super_block *sb)
>>> if (inode) {
>>> inode->i_ino = get_next_ino();
>>> inode_init_owner(&nop_mnt_idmap, inode, NULL, mode);
>>> - inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
>>> + inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
>>> inc_nlink(inode);
>>>
>>> inode->i_fop = &simple_dir_operations;
>>> @@ -360,7 +360,7 @@ static struct inode *dlmfs_get_inode(struct inode *parent,
>>>
>>> inode->i_ino = get_next_ino();
>>> inode_init_owner(&nop_mnt_idmap, inode, parent, mode);
>>> - inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
>>> + inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
>>>
>>> ip = DLMFS_I(inode);
>>> ip->ip_conn = DLMFS_I(parent)->ip_conn;
>>> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
>>> index c28bc983a7b1..c3e2961ee5db 100644
>>> --- a/fs/ocfs2/dlmglue.c
>>> +++ b/fs/ocfs2/dlmglue.c
>>> @@ -2162,6 +2162,7 @@ static void __ocfs2_stuff_meta_lvb(struct inode *inode)
>>> struct ocfs2_inode_info *oi = OCFS2_I(inode);
>>> struct ocfs2_lock_res *lockres = &oi->ip_inode_lockres;
>>> struct ocfs2_meta_lvb *lvb;
>>> + struct timespec64 ctime = inode_get_ctime(inode);
>>>
>>> lvb = ocfs2_dlm_lvb(&lockres->l_lksb);
>>>
>>> @@ -2185,7 +2186,7 @@ static void __ocfs2_stuff_meta_lvb(struct inode *inode)
>>> lvb->lvb_iatime_packed =
>>> cpu_to_be64(ocfs2_pack_timespec(&inode->i_atime));
>>> lvb->lvb_ictime_packed =
>>> - cpu_to_be64(ocfs2_pack_timespec(&inode->i_ctime));
>>> + cpu_to_be64(ocfs2_pack_timespec(&ctime));
>>> lvb->lvb_imtime_packed =
>>> cpu_to_be64(ocfs2_pack_timespec(&inode->i_mtime));
>>> lvb->lvb_iattr = cpu_to_be32(oi->ip_attr);
>>> @@ -2208,6 +2209,7 @@ static int ocfs2_refresh_inode_from_lvb(struct inode *inode)
>>> struct ocfs2_inode_info *oi = OCFS2_I(inode);
>>> struct ocfs2_lock_res *lockres = &oi->ip_inode_lockres;
>>> struct ocfs2_meta_lvb *lvb;
>>> + struct timespec64 ctime;
>>>
>>> mlog_meta_lvb(0, lockres);
>>>
>>> @@ -2238,8 +2240,9 @@ static int ocfs2_refresh_inode_from_lvb(struct inode *inode)
>>> be64_to_cpu(lvb->lvb_iatime_packed));
>>> ocfs2_unpack_timespec(&inode->i_mtime,
>>> be64_to_cpu(lvb->lvb_imtime_packed));
>>> - ocfs2_unpack_timespec(&inode->i_ctime,
>>> + ocfs2_unpack_timespec(&ctime,
>>> be64_to_cpu(lvb->lvb_ictime_packed));
>>> + inode_set_ctime_to_ts(inode, ctime);
>>
>> A quick glance, it seems not an equivalent replace.
>>
>
>
> How so?
>
> The old code unpacked the time directly into the inode->i_ctime. The new
> one unpacks it into a local timespec64 variable and then sets the
> inode->i_ctime to that value. The result should still be the same.
>
IC, it looks fine to me.
Acked-by: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>