| Message ID | 20180620143448.44388-3-arnd@arndb.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] btrfs: use monotonic time for transaction handling | expand |
On Wed, Jun 20, 2018 at 04:34:34PM +0200, Arnd Bergmann wrote: > While the regular inode timestamps all use timespec64 now, the > i_otime field is btrfs specific and still needs to be converted > to correctly represent times beyond 2038. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> This patch addresses the remaining type conversions, so I'm going to merge it, thanks.
On 20.06.2018 19:38, David Sterba wrote: > On Wed, Jun 20, 2018 at 04:34:34PM +0200, Arnd Bergmann wrote: >> While the regular inode timestamps all use timespec64 now, the >> i_otime field is btrfs specific and still needs to be converted >> to correctly represent times beyond 2038. >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > This patch addresses the remaining type conversions, so I'm going to > merge it, thanks. > Actually for the sake of consistency we might want to merge this series altogether. As it stands we now use ktime_get_seconds and ktime_get_real_seconds (from Allen's patch). I haven't dug to see what's the difference (if any) between the two .
On Wed, Jun 20, 2018 at 9:34 PM, Nikolay Borisov <nborisov@suse.com> wrote: > > > On 20.06.2018 19:38, David Sterba wrote: >> On Wed, Jun 20, 2018 at 04:34:34PM +0200, Arnd Bergmann wrote: >>> While the regular inode timestamps all use timespec64 now, the >>> i_otime field is btrfs specific and still needs to be converted >>> to correctly represent times beyond 2038. >>> >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> >> This patch addresses the remaining type conversions, so I'm going to >> merge it, thanks. >> > > Actually for the sake of consistency we might want to merge this series > altogether. As it stands we now use ktime_get_seconds and > ktime_get_real_seconds (from Allen's patch). I haven't dug to see what's > the difference (if any) between the two . I just checked again and see that Allen's patch addresses the first two of my three patches, but he picked a different approach for transaction_kthread(): My patch moved to CLOCK_MONOTONIC, while his version only changed the to time64_t but kept the CLOCK_REALTIME behavior. It's a small difference, but I think my version is slightly better. My patch 2/3 is identical to his version. If you like, I can also rebase my patch 1/3 on top of his patch and change it to CLOCK_MONOTONIC. Arnd
On 20.06.2018 22:41, Arnd Bergmann wrote: > On Wed, Jun 20, 2018 at 9:34 PM, Nikolay Borisov <nborisov@suse.com> wrote: >> >> >> On 20.06.2018 19:38, David Sterba wrote: >>> On Wed, Jun 20, 2018 at 04:34:34PM +0200, Arnd Bergmann wrote: >>>> While the regular inode timestamps all use timespec64 now, the >>>> i_otime field is btrfs specific and still needs to be converted >>>> to correctly represent times beyond 2038. >>>> >>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>> >>> This patch addresses the remaining type conversions, so I'm going to >>> merge it, thanks. >>> >> >> Actually for the sake of consistency we might want to merge this series >> altogether. As it stands we now use ktime_get_seconds and >> ktime_get_real_seconds (from Allen's patch). I haven't dug to see what's >> the difference (if any) between the two . > > I just checked again and see that Allen's patch addresses the first two > of my three patches, but he picked a different approach for > transaction_kthread(): My patch moved to CLOCK_MONOTONIC, > while his version only changed the to time64_t but kept the > CLOCK_REALTIME behavior. It's a small difference, but I think > my version is slightly better. My patch 2/3 is identical to his version. I agree, in the transaction_kthread we are only interested in knowing whether a fixed time windows (commit_internval) has passed. So monotonic makes more sense here. > > If you like, I can also rebase my patch 1/3 on top of his patch and > change it to CLOCK_MONOTONIC. Please do. > > Arnd >
diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h index 7e075343daa5..1343ac57b438 100644 --- a/fs/btrfs/btrfs_inode.h +++ b/fs/btrfs/btrfs_inode.h @@ -178,7 +178,7 @@ struct btrfs_inode { struct btrfs_delayed_node *delayed_node; /* File creation time. */ - struct timespec i_otime; + struct timespec64 i_otime; /* Hook into fs_info->delayed_iputs */ struct list_head delayed_iput; diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index e9482f0db9d0..22dcc8afd38f 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5745,7 +5745,7 @@ static struct inode *new_simple_dir(struct super_block *s, inode->i_mtime = current_time(inode); inode->i_atime = inode->i_mtime; inode->i_ctime = inode->i_mtime; - BTRFS_I(inode)->i_otime = timespec64_to_timespec(inode->i_mtime); + BTRFS_I(inode)->i_otime = inode->i_mtime; return inode; } @@ -6349,7 +6349,7 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans, inode->i_mtime = current_time(inode); inode->i_atime = inode->i_mtime; inode->i_ctime = inode->i_mtime; - BTRFS_I(inode)->i_otime = timespec64_to_timespec(inode->i_mtime); + BTRFS_I(inode)->i_otime = inode->i_mtime; inode_item = btrfs_item_ptr(path->nodes[0], path->slots[0], struct btrfs_inode_item);
While the regular inode timestamps all use timespec64 now, the i_otime field is btrfs specific and still needs to be converted to correctly represent times beyond 2038. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/btrfs/btrfs_inode.h | 2 +- fs/btrfs/inode.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.9.0