Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1430957 > unrolled thread

Re: [PATCH v2 07/24] fs: ubifs: Replace CURRENT_TIME_SEC with current_time

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 07/24] fs: ubifs: Replace CURRENT_TIME_SEC with current_time Deepa Dinamani <deepa.kernel@gmail.com> - 2016-06-24 23:10 +0200
    Re: [Y2038] [PATCH v2 07/24] fs: ubifs: Replace CURRENT_TIME_SEC with current_time Arnd Bergmann <arnd@arndb.de> - 2016-06-24 23:20 +0200

#1430957 — Re: [PATCH v2 07/24] fs: ubifs: Replace CURRENT_TIME_SEC with current_time

>> @@ -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

[toc] | [next] | [standalone]

#1430961 — Re: [Y2038] [PATCH v2 07/24] fs: ubifs: Replace CURRENT_TIME_SEC with current_time

On Friday, June 24, 2016 2:05:11 PM CEST Deepa Dinamani wrote:
> > 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.

It is a bit odd, but I can't think of why that would be a problem.
All the other instances have to wait until the inode timestamps
are converted but this one does not.

> 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.

I'd leave it in.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web