Re: [PATCH v2 07/24] fs: ubifs: Replace CURRENT_TIME_SEC with current_time
From: Deepa Dinamani
Date: Fri Jun 24 2016 - 17:05:20 EST
>> @@ -84,6 +84,8 @@ static int create_default_filesystem(struct ubifs_info *c)
>> int min_leb_cnt = UBIFS_MIN_LEB_CNT;
>> long long tmp64, main_bytes;
>> __le64 tmp_le64;
>> + __le32 tmp_le32;
>> + struct timespec ts;
>>
>> /* Some functions called from here depend on the @c->key_len filed */
>> c->key_len = UBIFS_SK_LEN;
>> @@ -297,13 +299,17 @@ static int create_default_filesystem(struct ubifs_info *c)
>> ino->ch.node_type = UBIFS_INO_NODE;
>> ino->creat_sqnum = cpu_to_le64(++c->max_sqnum);
>> ino->nlink = cpu_to_le32(2);
>> - tmp_le64 = cpu_to_le64(CURRENT_TIME_SEC.tv_sec);
>> +
>> + ktime_get_real_ts(&ts);
>> + ts = timespec_trunc(ts, DEFAULT_TIME_GRAN);
>> + tmp_le64 = cpu_to_le64(ts.tv_sec);
>> ino->atime_sec = tmp_le64;
>> ino->ctime_sec = tmp_le64;
>> ino->mtime_sec = tmp_le64;
>> - ino->atime_nsec = 0;
>> - ino->ctime_nsec = 0;
>> - ino->mtime_nsec = 0;
>> + tmp_le32 = cpu_to_le32(ts.tv_nsec);
>> + ino->atime_nsec = tmp_le32;
>> + ino->ctime_nsec = tmp_le32;
>> + ino->mtime_nsec = tmp_le32;
>
> This part of the patch seems independent of the rest, as you don't actually
> use current_time() here, or assign the timespec to an inode.
>
> I'd suggest either leaving this part out of the patch series for now,
> or making it a separate patch that uses timespec64 directly.
This is actually the root inode which is created and written to disk.
We actually want to use current_time() here, but this is not cached.
So we don't have a vfs inode.
struct ubifs_ino_node represents inode format on the disk.
I thought it would be odd to fill this with timespec64 only here.
My plan was to switch it over to timespec64 when all of ubifs changes
to use timespec64.
This also was helping the current series as it let me delete
CURRENT_TIME macros.
I can add a comment to suggest this in code.
But, what you suggest should also work fine since the on disk
representation is big enough to use timespec64 already.
Let me know if you want me to drop this change for now as we delete
CURRENT_TIME macros after rc1 now.
-Deepa