diff mbox

[3/3] btrfs: use timespec64 for i_otime

Message ID 20180620143448.44388-3-arnd@arndb.de (mailing list archive)
State Accepted
Headers show

Commit Message

Arnd Bergmann June 20, 2018, 2:34 p.m. UTC
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(-)

Comments

David Sterba June 20, 2018, 4:38 p.m. UTC | #1
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nikolay Borisov June 20, 2018, 7:34 p.m. UTC | #2
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 .
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann June 20, 2018, 7:41 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nikolay Borisov June 21, 2018, 8:23 a.m. UTC | #4
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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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