2020-01-15 08:30:15

by Namjae Jeon

[permalink] [raw]
Subject: [PATCH v10 09/14] exfat: add misc operations

This adds the implementation of misc operations for exfat.

Signed-off-by: Namjae Jeon <[email protected]>
Signed-off-by: Sungjong Seo <[email protected]>
---
fs/exfat/misc.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 162 insertions(+)
create mode 100644 fs/exfat/misc.c

diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c
new file mode 100644
index 000000000000..ed3dae7b54c3
--- /dev/null
+++ b/fs/exfat/misc.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Written 1992,1993 by Werner Almesberger
+ * 22/11/2000 - Fixed fat_date_unix2dos for dates earlier than 01/01/1980
+ * and date_dos2unix for date==0 by Igor Zhbanov([email protected])
+ * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd.
+ */
+
+#include <linux/time.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/buffer_head.h>
+
+#include "exfat_raw.h"
+#include "exfat_fs.h"
+
+/*
+ * exfat_fs_error reports a file system problem that might indicate fa data
+ * corruption/inconsistency. Depending on 'errors' mount option the
+ * panic() is called, or error message is printed FAT and nothing is done,
+ * or filesystem is remounted read-only (default behavior).
+ * In case the file system is remounted read-only, it can be made writable
+ * again by remounting it.
+ */
+void __exfat_fs_error(struct super_block *sb, int report, const char *fmt, ...)
+{
+ struct exfat_mount_options *opts = &EXFAT_SB(sb)->options;
+ va_list args;
+ struct va_format vaf;
+
+ if (report) {
+ va_start(args, fmt);
+ vaf.fmt = fmt;
+ vaf.va = &args;
+ exfat_msg(sb, KERN_ERR, "error, %pV\n", &vaf);
+ va_end(args);
+ }
+
+ if (opts->errors == EXFAT_ERRORS_PANIC) {
+ panic("exFAT-fs (%s): fs panic from previous error\n",
+ sb->s_id);
+ } else if (opts->errors == EXFAT_ERRORS_RO && !sb_rdonly(sb)) {
+ sb->s_flags |= SB_RDONLY;
+ exfat_msg(sb, KERN_ERR, "Filesystem has been set read-only");
+ }
+}
+
+/*
+ * exfat_msg() - print preformated EXFAT specific messages.
+ * All logs except what uses exfat_fs_error() should be written by exfat_msg()
+ */
+void exfat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ va_start(args, fmt);
+ vaf.fmt = fmt;
+ vaf.va = &args;
+ /* level means KERN_ pacility level */
+ printk("%sexFAT-fs (%s): %pV\n", level, sb->s_id, &vaf);
+ va_end(args);
+}
+
+#define SECS_PER_MIN (60)
+#define TIMEZONE_SEC(x) ((x) * 15 * SECS_PER_MIN)
+
+static void exfat_adjust_tz(struct timespec64 *ts, u8 tz_off)
+{
+ if (tz_off <= 0x3F)
+ ts->tv_sec -= TIMEZONE_SEC(tz_off);
+ else /* 0x40 <= (tz_off & 0x7F) <=0x7F */
+ ts->tv_sec += TIMEZONE_SEC(0x80 - tz_off);
+}
+
+static inline int exfat_tz_offset(struct exfat_sb_info *sbi)
+{
+ if (sbi->options.time_offset)
+ return sbi->options.time_offset;
+ return sys_tz.tz_minuteswest;
+}
+
+/* Convert a EXFAT time/date pair to a UNIX date (seconds since 1 1 70). */
+void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
+ __le16 time, __le16 date, u8 tz)
+{
+ u16 t = le16_to_cpu(time);
+ u16 d = le16_to_cpu(date);
+
+ ts->tv_sec = mktime64(1980 + (d >> 9), d >> 5 & 0x000F, d & 0x001F,
+ t >> 11, (t >> 5) & 0x003F, (t & 0x001F) << 1);
+ ts->tv_nsec = 0;
+
+ if (tz & EXFAT_TZ_VALID)
+ /* Treat as UTC time, but need to adjust timezone to UTC0 */
+ exfat_adjust_tz(ts, tz & ~EXFAT_TZ_VALID);
+ else
+ /* Treat as local time */
+ ts->tv_sec -= exfat_tz_offset(sbi) * SECS_PER_MIN;
+}
+
+/* Convert linear UNIX date to a EXFAT time/date pair. */
+void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
+ __le16 *time, __le16 *date, u8 *tz)
+{
+ struct tm tm;
+ u16 t, d;
+
+ /* clamp to the range valid in the exfat on-disk representation. */
+ time64_to_tm(clamp_t(time64_t, ts->tv_sec, EXFAT_MIN_TIMESTAMP_SECS,
+ EXFAT_MAX_TIMESTAMP_SECS), -exfat_tz_offset(sbi) * SECS_PER_MIN,
+ &tm);
+ t = (tm.tm_hour << 11) | (tm.tm_min << 5) | (tm.tm_sec >> 1);
+ d = ((tm.tm_year - 80) << 9) | ((tm.tm_mon + 1) << 5) | tm.tm_mday;
+
+ *time = cpu_to_le16(t);
+ *date = cpu_to_le16(d);
+
+ /*
+ * exfat ondisk tz offset field decribes the offset from UTF
+ * in 15 minute interval.
+ */
+ *tz = ((exfat_tz_offset(sbi) / -15) & 0x7F) | EXFAT_TZ_VALID;
+}
+
+unsigned short exfat_calc_chksum_2byte(void *data, int len,
+ unsigned short chksum, int type)
+{
+ int i;
+ unsigned char *c = (unsigned char *)data;
+
+ for (i = 0; i < len; i++, c++) {
+ if (((i == 2) || (i == 3)) && (type == CS_DIR_ENTRY))
+ continue;
+ chksum = (((chksum & 1) << 15) | ((chksum & 0xFFFE) >> 1)) +
+ (unsigned short)*c;
+ }
+ return chksum;
+}
+
+void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync)
+{
+ set_bit(EXFAT_SB_DIRTY, &EXFAT_SB(sb)->s_state);
+ set_buffer_uptodate(bh);
+ mark_buffer_dirty(bh);
+
+ if (sync)
+ sync_dirty_buffer(bh);
+}
+
+void exfat_chain_set(struct exfat_chain *ec, unsigned int dir,
+ unsigned int size, unsigned char flags)
+{
+ ec->dir = dir;
+ ec->size = size;
+ ec->flags = flags;
+}
+
+void exfat_chain_dup(struct exfat_chain *dup, struct exfat_chain *ec)
+{
+ return exfat_chain_set(dup, ec->dir, ec->size, ec->flags);
+}
--
2.17.1


2020-01-15 10:12:19

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Wed, Jan 15, 2020 at 9:28 AM Namjae Jeon <[email protected]> wrote:

> +#define SECS_PER_MIN (60)
> +#define TIMEZONE_SEC(x) ((x) * 15 * SECS_PER_MIN)
> +
> +static void exfat_adjust_tz(struct timespec64 *ts, u8 tz_off)
> +{
> + if (tz_off <= 0x3F)
> + ts->tv_sec -= TIMEZONE_SEC(tz_off);
> + else /* 0x40 <= (tz_off & 0x7F) <=0x7F */
> + ts->tv_sec += TIMEZONE_SEC(0x80 - tz_off);
> +}
> +
> +static inline int exfat_tz_offset(struct exfat_sb_info *sbi)
> +{
> + if (sbi->options.time_offset)
> + return sbi->options.time_offset;
> + return sys_tz.tz_minuteswest;
> +}
> +
> +/* Convert a EXFAT time/date pair to a UNIX date (seconds since 1 1 70). */
> +void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
> + __le16 time, __le16 date, u8 tz)
> +{
> + u16 t = le16_to_cpu(time);
> + u16 d = le16_to_cpu(date);
> +
> + ts->tv_sec = mktime64(1980 + (d >> 9), d >> 5 & 0x000F, d & 0x001F,
> + t >> 11, (t >> 5) & 0x003F, (t & 0x001F) << 1);
> + ts->tv_nsec = 0;

This part looks good to me now.

> + if (tz & EXFAT_TZ_VALID)
> + /* Treat as UTC time, but need to adjust timezone to UTC0 */
> + exfat_adjust_tz(ts, tz & ~EXFAT_TZ_VALID);
> + else
> + /* Treat as local time */
> + ts->tv_sec -= exfat_tz_offset(sbi) * SECS_PER_MIN;
> +}

Whereas this seems rather complex, when it deals with three different
cases:

- timezone stored in inode
- timezone offset passed as mount option
- local time from sys_tz.tz_minuteswest

Does the exfat specification require to use some notion of 'local time' here
as the fallback? The problem with sys_tz.tz_minuteswest is that it is
not too well-defined, so if there is a choice, falling back to UTC would
be nicer.

> +/* Convert linear UNIX date to a EXFAT time/date pair. */
> +void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
> + __le16 *time, __le16 *date, u8 *tz)
> +{
> + struct tm tm;
> + u16 t, d;
> +
> + /* clamp to the range valid in the exfat on-disk representation. */
> + time64_to_tm(clamp_t(time64_t, ts->tv_sec, EXFAT_MIN_TIMESTAMP_SECS,
> + EXFAT_MAX_TIMESTAMP_SECS), -exfat_tz_offset(sbi) * SECS_PER_MIN,
> + &tm);

I think you can drop the clamping here, as thes_time_min/s_time_max fields
should take care of that.

For writing out timestamps, it may be best to always encode them as UTC
and set set timezone-valid bit for that. That way, the min/max values
are known at compile time regardless of which time zone the machine
thinks it is in.

Arnd

2020-01-15 13:32:22

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

2020-01-15 19:10 GMT+09:00, Arnd Bergmann <[email protected]>:
> On Wed, Jan 15, 2020 at 9:28 AM Namjae Jeon <[email protected]>
> wrote:
>
>> +#define SECS_PER_MIN (60)
>> +#define TIMEZONE_SEC(x) ((x) * 15 * SECS_PER_MIN)
>> +
>> +static void exfat_adjust_tz(struct timespec64 *ts, u8 tz_off)
>> +{
>> + if (tz_off <= 0x3F)
>> + ts->tv_sec -= TIMEZONE_SEC(tz_off);
>> + else /* 0x40 <= (tz_off & 0x7F) <=0x7F */
>> + ts->tv_sec += TIMEZONE_SEC(0x80 - tz_off);
>> +}
>> +
>> +static inline int exfat_tz_offset(struct exfat_sb_info *sbi)
>> +{
>> + if (sbi->options.time_offset)
>> + return sbi->options.time_offset;
>> + return sys_tz.tz_minuteswest;
>> +}
>> +
>> +/* Convert a EXFAT time/date pair to a UNIX date (seconds since 1 1 70).
>> */
>> +void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64
>> *ts,
>> + __le16 time, __le16 date, u8 tz)
>> +{
>> + u16 t = le16_to_cpu(time);
>> + u16 d = le16_to_cpu(date);
>> +
>> + ts->tv_sec = mktime64(1980 + (d >> 9), d >> 5 & 0x000F, d &
>> 0x001F,
>> + t >> 11, (t >> 5) & 0x003F, (t & 0x001F) <<
>> 1);
>> + ts->tv_nsec = 0;
>
> This part looks good to me now.
Thanks.
>
>> + if (tz & EXFAT_TZ_VALID)
>> + /* Treat as UTC time, but need to adjust timezone to UTC0
>> */
>> + exfat_adjust_tz(ts, tz & ~EXFAT_TZ_VALID);
>> + else
>> + /* Treat as local time */
>> + ts->tv_sec -= exfat_tz_offset(sbi) * SECS_PER_MIN;
>> +}
>
> Whereas this seems rather complex, when it deals with three different
> cases:
>
> - timezone stored in inode
> - timezone offset passed as mount option
> - local time from sys_tz.tz_minuteswest
>
> Does the exfat specification require to use some notion of 'local time'
> here
> as the fallback? The problem with sys_tz.tz_minuteswest is that it is
> not too well-defined,
It is not described in the specification. I don't know exactly what
the problem is because sys_tz.tz_minuteswest seems to work fine to me.
It can be random garbage value ?
> so if there is a choice, falling back to UTC would
> be nicer.
Okay.
>
>> +/* Convert linear UNIX date to a EXFAT time/date pair. */
>> +void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64
>> *ts,
>> + __le16 *time, __le16 *date, u8 *tz)
>> +{
>> + struct tm tm;
>> + u16 t, d;
>> +
>> + /* clamp to the range valid in the exfat on-disk representation.
>> */
>> + time64_to_tm(clamp_t(time64_t, ts->tv_sec,
>> EXFAT_MIN_TIMESTAMP_SECS,
>> + EXFAT_MAX_TIMESTAMP_SECS), -exfat_tz_offset(sbi) *
>> SECS_PER_MIN,
>> + &tm);
>
> I think you can drop the clamping here, as thes_time_min/s_time_max fields
> should take care of that.
Okay.
>
> For writing out timestamps, it may be best to always encode them as UTC
> and set set timezone-valid bit for that. That way, the min/max values
> are known at compile time regardless of which time zone the machine
> thinks it is in.
Okay, I will check it.
Thanks for your review!
>
> Arnd
>

2020-01-15 13:40:10

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Wednesday 15 January 2020 22:30:59 Namjae Jeon wrote:
> 2020-01-15 19:10 GMT+09:00, Arnd Bergmann <[email protected]>:
> > On Wed, Jan 15, 2020 at 9:28 AM Namjae Jeon <[email protected]>
> > wrote:
> >
> >> +#define SECS_PER_MIN (60)
> >> +#define TIMEZONE_SEC(x) ((x) * 15 * SECS_PER_MIN)
> >> +
> >> +static void exfat_adjust_tz(struct timespec64 *ts, u8 tz_off)
> >> +{
> >> + if (tz_off <= 0x3F)
> >> + ts->tv_sec -= TIMEZONE_SEC(tz_off);
> >> + else /* 0x40 <= (tz_off & 0x7F) <=0x7F */
> >> + ts->tv_sec += TIMEZONE_SEC(0x80 - tz_off);
> >> +}
> >> +
> >> +static inline int exfat_tz_offset(struct exfat_sb_info *sbi)
> >> +{
> >> + if (sbi->options.time_offset)
> >> + return sbi->options.time_offset;
> >> + return sys_tz.tz_minuteswest;
> >> +}
> >> +
> >> +/* Convert a EXFAT time/date pair to a UNIX date (seconds since 1 1 70).
> >> */
> >> +void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64
> >> *ts,
> >> + __le16 time, __le16 date, u8 tz)
> >> +{
> >> + u16 t = le16_to_cpu(time);
> >> + u16 d = le16_to_cpu(date);
> >> +
> >> + ts->tv_sec = mktime64(1980 + (d >> 9), d >> 5 & 0x000F, d &
> >> 0x001F,
> >> + t >> 11, (t >> 5) & 0x003F, (t & 0x001F) <<
> >> 1);
> >> + ts->tv_nsec = 0;
> >
> > This part looks good to me now.
> Thanks.
> >
> >> + if (tz & EXFAT_TZ_VALID)
> >> + /* Treat as UTC time, but need to adjust timezone to UTC0
> >> */
> >> + exfat_adjust_tz(ts, tz & ~EXFAT_TZ_VALID);
> >> + else
> >> + /* Treat as local time */
> >> + ts->tv_sec -= exfat_tz_offset(sbi) * SECS_PER_MIN;
> >> +}
> >
> > Whereas this seems rather complex, when it deals with three different
> > cases:
> >
> > - timezone stored in inode
> > - timezone offset passed as mount option
> > - local time from sys_tz.tz_minuteswest
> >
> > Does the exfat specification require to use some notion of 'local time'
> > here
> > as the fallback? The problem with sys_tz.tz_minuteswest is that it is
> > not too well-defined,
> It is not described in the specification. I don't know exactly what
> the problem is because sys_tz.tz_minuteswest seems to work fine to me.
> It can be random garbage value ?
> > so if there is a choice, falling back to UTC would
> > be nicer.
> Okay.

Arnd, what is the default value of sys_tz.tz_minuteswest? What is the
benefit of not using it?

I though that timezone mount option is just an old hack when userspace
does not correctly set kernel's timezone and that this timezone mount
option should be in most cases avoided.

So also another question, what is benefit of having fs specific timezone
mount option? As it is fs specific it means that it would be used so
much.

> >
> >> +/* Convert linear UNIX date to a EXFAT time/date pair. */
> >> +void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64
> >> *ts,
> >> + __le16 *time, __le16 *date, u8 *tz)
> >> +{
> >> + struct tm tm;
> >> + u16 t, d;
> >> +
> >> + /* clamp to the range valid in the exfat on-disk representation.
> >> */
> >> + time64_to_tm(clamp_t(time64_t, ts->tv_sec,
> >> EXFAT_MIN_TIMESTAMP_SECS,
> >> + EXFAT_MAX_TIMESTAMP_SECS), -exfat_tz_offset(sbi) *
> >> SECS_PER_MIN,
> >> + &tm);
> >
> > I think you can drop the clamping here, as thes_time_min/s_time_max fields
> > should take care of that.
> Okay.
> >
> > For writing out timestamps, it may be best to always encode them as UTC
> > and set set timezone-valid bit for that. That way, the min/max values
> > are known at compile time regardless of which time zone the machine
> > thinks it is in.
> Okay, I will check it.
> Thanks for your review!
> >
> > Arnd
> >

--
Pali Rohár
[email protected]

2020-01-15 13:52:42

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Wed, Jan 15, 2020 at 2:38 PM Pali Rohár <[email protected]> wrote:
> On Wednesday 15 January 2020 22:30:59 Namjae Jeon wrote:
> > 2020-01-15 19:10 GMT+09:00, Arnd Bergmann <[email protected]>:

> > It is not described in the specification. I don't know exactly what
> > the problem is because sys_tz.tz_minuteswest seems to work fine to me.
> > It can be random garbage value ?
> >
> > > so if there is a choice, falling back to UTC would
> > > be nicer.
> >
> > Okay.
>
> Arnd, what is the default value of sys_tz.tz_minuteswest? What is the
> benefit of not using it?
>
> I though that timezone mount option is just an old hack when userspace
> does not correctly set kernel's timezone and that this timezone mount
> option should be in most cases avoided.

The main problem is that it is system-wide and initialized at boot
time through settimeofday() to a timezone picked by the system
administrator.

However, in user space, every user may set their own timezone with
the 'TZ' variable, and the default timezone may be different inside of a
container based on the contents of /etc/timezone in its root directory.

> So also another question, what is benefit of having fs specific timezone
> mount option? As it is fs specific it means that it would be used so
> much.

You can use it to access removable media that were written in
a different timezone, or a partition that is shared with another OS
running on the same machine but with different timezone settings.

Arnd

2020-01-15 14:25:54

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Wednesday 15 January 2020 14:50:10 Arnd Bergmann wrote:
> On Wed, Jan 15, 2020 at 2:38 PM Pali Rohár <[email protected]> wrote:
> > On Wednesday 15 January 2020 22:30:59 Namjae Jeon wrote:
> > > 2020-01-15 19:10 GMT+09:00, Arnd Bergmann <[email protected]>:
>
> > > It is not described in the specification. I don't know exactly what
> > > the problem is because sys_tz.tz_minuteswest seems to work fine to me.
> > > It can be random garbage value ?
> > >
> > > > so if there is a choice, falling back to UTC would
> > > > be nicer.
> > >
> > > Okay.
> >
> > Arnd, what is the default value of sys_tz.tz_minuteswest? What is the
> > benefit of not using it?
> >
> > I though that timezone mount option is just an old hack when userspace
> > does not correctly set kernel's timezone and that this timezone mount
> > option should be in most cases avoided.
>
> The main problem is that it is system-wide and initialized at boot
> time through settimeofday() to a timezone picked by the system
> administrator.
>
> However, in user space, every user may set their own timezone with
> the 'TZ' variable, and the default timezone may be different inside of a
> container based on the contents of /etc/timezone in its root directory.

So kernel timezone is shared across all containers, right?

> > So also another question, what is benefit of having fs specific timezone
> > mount option? As it is fs specific it means that it would be used so
> > much.
>
> You can use it to access removable media that were written in
> a different timezone or a partition that is shared with another OS
> running on the same machine but with different timezone settings.

So... basically all userspace <--> kernel API which works with timestamp
do not have information about timezone right? creat(), utime() or
utimensat() just pass timestamp without timezone information. Is this
timestamp mean to be in UTC or in local user time zone (as specified by
user's TZ= env variable)?

--
Pali Rohár
[email protected]

2020-01-15 15:05:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Wed, Jan 15, 2020 at 3:24 PM Pali Rohár <[email protected]> wrote:
> On Wednesday 15 January 2020 14:50:10 Arnd Bergmann wrote:
> > On Wed, Jan 15, 2020 at 2:38 PM Pali Rohár <[email protected]> wrote:
> > > On Wednesday 15 January 2020 22:30:59 Namjae Jeon wrote:
> > > > 2020-01-15 19:10 GMT+09:00, Arnd Bergmann <[email protected]>:
> > However, in user space, every user may set their own timezone with
> > the 'TZ' variable, and the default timezone may be different inside of a
> > container based on the contents of /etc/timezone in its root directory.
>
> So kernel timezone is shared across all containers, right?

Yes.

> > You can use it to access removable media that were written in
> > a different timezone or a partition that is shared with another OS
> > running on the same machine but with different timezone settings.
>
> So... basically all userspace <--> kernel API which works with timestamp
> do not have information about timezone right? creat(), utime() or
> utimensat() just pass timestamp without timezone information. Is this
> timestamp mean to be in UTC or in local user time zone (as specified by
> user's TZ= env variable)?

As a rule, all timekeeping in the kernel is done in terms of UTC. You can
see what the current exceptions are using

$ git grep -wl sys_tz
Documentation/filesystems/vfat.txt
arch/alpha/kernel/osf_sys.c
arch/nds32/kernel/vdso.c
arch/powerpc/kernel/time.c
arch/s390/kernel/time.c
arch/sparc/kernel/vdso.c
drivers/media/platform/vivid/vivid-rds-gen.c
drivers/media/platform/vivid/vivid-vbi-gen.c
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
drivers/scsi/3w-9xxx.c
drivers/scsi/3w-sas.c
drivers/scsi/aacraid/commsup.c
drivers/scsi/arcmsr/arcmsr_hba.c
drivers/scsi/mvumi.c
drivers/scsi/mvumi.h
drivers/scsi/smartpqi/smartpqi_init.c
fs/affs/amigaffs.c
fs/affs/inode.c
fs/affs/super.c
fs/fat/misc.c
fs/hfs/hfs_fs.h
fs/hfs/inode.c
fs/hfs/sysdep.c
fs/hpfs/hpfs_fn.h
fs/udf/udftime.c
include/linux/time.h
kernel/debug/kdb/kdb_main.c
kernel/time/ntp.c
kernel/time/time.c
kernel/time/timekeeping.c
kernel/time/vsyscall.c
net/netfilter/nft_meta.c
net/netfilter/xt_time.c
tools/testing/selftests/x86/test_vdso.c

The vdso and kernel/time/ code are for maintaining the timezone through
settimeofday()/gettimeofday(), and the drivers should probably all be changed
to use UTC. The file systems (affs, fat, hfs, hpfs and udf) do this for
compatibility with other operating systems that store the metadata in
localtime.

Arnd

2020-01-15 15:42:02

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Wednesday 15 January 2020 16:03:12 Arnd Bergmann wrote:
> On Wed, Jan 15, 2020 at 3:24 PM Pali Rohár <[email protected]> wrote:
> > On Wednesday 15 January 2020 14:50:10 Arnd Bergmann wrote:
> > > On Wed, Jan 15, 2020 at 2:38 PM Pali Rohár <[email protected]> wrote:
> > > > On Wednesday 15 January 2020 22:30:59 Namjae Jeon wrote:
> > > > > 2020-01-15 19:10 GMT+09:00, Arnd Bergmann <[email protected]>:
> > > However, in user space, every user may set their own timezone with
> > > the 'TZ' variable, and the default timezone may be different inside of a
> > > container based on the contents of /etc/timezone in its root directory.
> >
> > So kernel timezone is shared across all containers, right?
>
> Yes.
>
> > > You can use it to access removable media that were written in
> > > a different timezone or a partition that is shared with another OS
> > > running on the same machine but with different timezone settings.
> >
> > So... basically all userspace <--> kernel API which works with timestamp
> > do not have information about timezone right? creat(), utime() or
> > utimensat() just pass timestamp without timezone information. Is this
> > timestamp mean to be in UTC or in local user time zone (as specified by
> > user's TZ= env variable)?
>
> As a rule, all timekeeping in the kernel is done in terms of UTC.

Ok, thanks!

> You can
> see what the current exceptions are using
>
> $ git grep -wl sys_tz
> Documentation/filesystems/vfat.txt
> arch/alpha/kernel/osf_sys.c
> arch/nds32/kernel/vdso.c
> arch/powerpc/kernel/time.c
> arch/s390/kernel/time.c
> arch/sparc/kernel/vdso.c
> drivers/media/platform/vivid/vivid-rds-gen.c
> drivers/media/platform/vivid/vivid-vbi-gen.c
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> drivers/scsi/3w-9xxx.c
> drivers/scsi/3w-sas.c
> drivers/scsi/aacraid/commsup.c
> drivers/scsi/arcmsr/arcmsr_hba.c
> drivers/scsi/mvumi.c
> drivers/scsi/mvumi.h
> drivers/scsi/smartpqi/smartpqi_init.c
> fs/affs/amigaffs.c
> fs/affs/inode.c
> fs/affs/super.c
> fs/fat/misc.c
> fs/hfs/hfs_fs.h
> fs/hfs/inode.c
> fs/hfs/sysdep.c
> fs/hpfs/hpfs_fn.h
> fs/udf/udftime.c
> include/linux/time.h
> kernel/debug/kdb/kdb_main.c
> kernel/time/ntp.c
> kernel/time/time.c
> kernel/time/timekeeping.c
> kernel/time/vsyscall.c
> net/netfilter/nft_meta.c
> net/netfilter/xt_time.c
> tools/testing/selftests/x86/test_vdso.c
>
> The vdso and kernel/time/ code are for maintaining the timezone through
> settimeofday()/gettimeofday(), and the drivers should probably all be changed
> to use UTC. The file systems (affs, fat, hfs, hpfs and udf) do this for
> compatibility with other operating systems that store the metadata in
> localtime.

Ok. But situation for exFAT is quite different. exFAT timestamp
structure contains also timezone information. Other filesystems do not
store timezone into their metadata (IIRC udf is exception and also
stores timezone). So question is in which timezone we should store to
exFAT timestamps. This is not for compatibility with legacy systems, but
because of fact that filesystem supports feature which is not common for
other filesystems.

Also, to make it more complicated exFAT supports storing timestamps also
in "unspecified" (local user) timezone, which matches other linux
filesystems.

So for storing timestamp we have options:

* Store them without timezone
* Store them in sys_tz timezone
* Store them in timezone specified in mount option
* Store them in UTC timezone

And when reading timestamp from exFAT we need to handle both:

* Timestamps without timezone
* Timestamps with timezone

So what is the best way to handle timezone/timestamps?

For me it looks sane:

When storing use: mount option timezone. When not available then use
sys_tz. And when sys_tz is not set (it is possible?), do not specify
timezone at all. Maybe there should be a mount option which says that
timestamps on exfat are stored without timezone.

When reading timestamp with timezone: Convert timestamp to timezone
specified in mount option (or fallback to sys_tz or fallback to UTC).

And when reading timestamp without timezone: Pass it as is without
conversion, ignoring all timezone mount options and sys_tz.

Arnd, what do you think about it?

--
Pali Rohár
[email protected]

2020-01-15 15:53:39

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Wed, Jan 15, 2020 at 4:39 PM Pali Rohár <[email protected]> wrote:
> On Wednesday 15 January 2020 16:03:12 Arnd Bergmann wrote:
> > The vdso and kernel/time/ code are for maintaining the timezone through
> > settimeofday()/gettimeofday(), and the drivers should probably all be changed
> > to use UTC. The file systems (affs, fat, hfs, hpfs and udf) do this for
> > compatibility with other operating systems that store the metadata in
> > localtime.
>
> Ok. But situation for exFAT is quite different. exFAT timestamp
> structure contains also timezone information. Other filesystems do not
> store timezone into their metadata (IIRC udf is exception and also
> stores timezone). So question is in which timezone we should store to
> exFAT timestamps. This is not for compatibility with legacy systems, but
> because of fact that filesystem supports feature which is not common for
> other filesystems.
>
> Also, to make it more complicated exFAT supports storing timestamps also
> in "unspecified" (local user) timezone, which matches other linux
> filesystems.
>
> So for storing timestamp we have options:
>
> * Store them without timezone
> * Store them in sys_tz timezone
> * Store them in timezone specified in mount option
> * Store them in UTC timezone
>
> And when reading timestamp from exFAT we need to handle both:
>
> * Timestamps without timezone
> * Timestamps with timezone

Right.

> So what is the best way to handle timezone/timestamps?
>
> For me it looks sane:
>
> When storing use: mount option timezone. When not available then use
> sys_tz. And when sys_tz is not set (it is possible?), do not specify
> timezone at all. Maybe there should be a mount option which says that
> timestamps on exfat are stored without timezone.

I would argue we should always store files in UTC, which seems to be
the most consistent with other file systems, and can be understood
by any other implementation that understands the timezone information
on disk or that expects UTC.

> When reading timestamp with timezone: Convert timestamp to timezone
> specified in mount option (or fallback to sys_tz or fallback to UTC).

Here I would just convert to UTC, which is what we store in the
in-memory struct inode anyway.

> And when reading timestamp without timezone: Pass it as is without
> conversion, ignoring all timezone mount options and sys_tz.
>
> Arnd, what do you think about it?

The last case (reading timestamp without timezone) is the only
one that I think we have to decide on: when reading an inode from
disk into memory, we have to convert it into UTC in some form.

This is what I think the timezone mount option should be used
for: if we don't know what the timezone was for the on-disk
timestamp, use the one provided by the user. However, if none
was specified, it should be either sys_tz or UTC (i.e. no
conversion). I would prefer the use of UTC here given the
problems with sys_tz, but sys_tz would be more consistent
with how fs/fat works.

Arnd

2020-01-16 10:26:17

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Wednesday 15 January 2020 16:52:12 Arnd Bergmann wrote:
> On Wed, Jan 15, 2020 at 4:39 PM Pali Rohár <[email protected]> wrote:
> > On Wednesday 15 January 2020 16:03:12 Arnd Bergmann wrote:
> > > The vdso and kernel/time/ code are for maintaining the timezone through
> > > settimeofday()/gettimeofday(), and the drivers should probably all be changed
> > > to use UTC. The file systems (affs, fat, hfs, hpfs and udf) do this for
> > > compatibility with other operating systems that store the metadata in
> > > localtime.
> >
> > Ok. But situation for exFAT is quite different. exFAT timestamp
> > structure contains also timezone information. Other filesystems do not
> > store timezone into their metadata (IIRC udf is exception and also
> > stores timezone). So question is in which timezone we should store to
> > exFAT timestamps. This is not for compatibility with legacy systems, but
> > because of fact that filesystem supports feature which is not common for
> > other filesystems.
> >
> > Also, to make it more complicated exFAT supports storing timestamps also
> > in "unspecified" (local user) timezone, which matches other linux
> > filesystems.
> >
> > So for storing timestamp we have options:
> >
> > * Store them without timezone
> > * Store them in sys_tz timezone
> > * Store them in timezone specified in mount option
> > * Store them in UTC timezone
> >
> > And when reading timestamp from exFAT we need to handle both:
> >
> > * Timestamps without timezone
> > * Timestamps with timezone
>
> Right.
>
> > So what is the best way to handle timezone/timestamps?
> >
> > For me it looks sane:
> >
> > When storing use: mount option timezone. When not available then use
> > sys_tz. And when sys_tz is not set (it is possible?), do not specify
> > timezone at all. Maybe there should be a mount option which says that
> > timestamps on exfat are stored without timezone.
>
> I would argue we should always store files in UTC, which seems to be
> the most consistent with other file systems, and can be understood
> by any other implementation that understands the timezone information
> on disk or that expects UTC.

exFAT timezone information needs to be understand by any exFAT
implementation.

I looked into exFAT specification and it has following information how
implementation should choose timezone:

https://docs.microsoft.com/en-us/windows/win32/fileio/exfat-specification#74101-offsetfromutc-field

However, implementations should only record the value 00h for this
field when:

1. Local date and time are actually the same as UTC, in which case
the value of the OffsetValid field shall be 1

2. Local date and time are not known, in which case the value of the
OffsetValid field shall be 1 and implementations shall consider
UTC to be local date and time

3. UTC is not known, in which case the value of the OffsetValid
field shall be 0

I'm not sure if storing time always in UTC is fine with specification.

Mount option which specify timezone can solve this problem, but only in
case when it is specified.

> > When reading timestamp with timezone: Convert timestamp to timezone
> > specified in mount option (or fallback to sys_tz or fallback to UTC).
>
> Here I would just convert to UTC, which is what we store in the
> in-memory struct inode anyway.

Ok. If inode timestamp is always in UTC, we should do same thing also
for exFAT.

> > And when reading timestamp without timezone: Pass it as is without
> > conversion, ignoring all timezone mount options and sys_tz.
> >
> > Arnd, what do you think about it?
>
> The last case (reading timestamp without timezone) is the only
> one that I think we have to decide on: when reading an inode from
> disk into memory, we have to convert it into UTC in some form.
>
> This is what I think the timezone mount option should be used
> for: if we don't know what the timezone was for the on-disk
> timestamp, use the one provided by the user.

I agree, this make sense.

> However, if none
> was specified, it should be either sys_tz or UTC (i.e. no
> conversion). I would prefer the use of UTC here given the
> problems with sys_tz, but sys_tz would be more consistent
> with how fs/fat works.

Hm... both UTC and sys_tz have positives and negatives. And I'm not
sure which option is better.

Do we know how Tuxera or Paragon solved this dilema in their exFAT
implementations? IIRC Paragon already sent their implementation to LKML.

--
Pali Rohár
[email protected]

2020-01-16 10:27:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Thu, Jan 16, 2020 at 11:19:47AM +0100, Pali Roh?r wrote:
> However, implementations should only record the value 00h for this
> field when:
>
> 1. Local date and time are actually the same as UTC, in which case
> the value of the OffsetValid field shall be 1
>
> 2. Local date and time are not known, in which case the value of the
> OffsetValid field shall be 1 and implementations shall consider
> UTC to be local date and time

Given time zones in Linux are per session I think our situation is
somewhat similar to 2.

> > Here I would just convert to UTC, which is what we store in the
> > in-memory struct inode anyway.
>
> Ok. If inode timestamp is always in UTC, we should do same thing also
> for exFAT.

> Hm... both UTC and sys_tz have positives and negatives. And I'm not
> sure which option is better.

The one big argument for always UTC is simplicity. Always using UTC
kills some arcane an unusual (for Linux file systems) code, and given
how exfat implementations deal with the time zone on reading should
always interoperate fine with other implementations.

2020-01-16 16:04:32

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Thursday 16 January 2020 11:23:07 Christoph Hellwig wrote:
> On Thu, Jan 16, 2020 at 11:19:47AM +0100, Pali Rohár wrote:
> > However, implementations should only record the value 00h for this
> > field when:
> >
> > 1. Local date and time are actually the same as UTC, in which case
> > the value of the OffsetValid field shall be 1
> >
> > 2. Local date and time are not known, in which case the value of the
> > OffsetValid field shall be 1 and implementations shall consider
> > UTC to be local date and time
>
> Given time zones in Linux are per session I think our situation is
> somewhat similar to 2.

Seems that 2. is really similar. Ok, storing by default in UTC make
sense, but still I'm thinking if there should be (or not) a mount option
which override default UTC timezone.

> > > Here I would just convert to UTC, which is what we store in the
> > > in-memory struct inode anyway.
> >
> > Ok. If inode timestamp is always in UTC, we should do same thing also
> > for exFAT.
>
> > Hm... both UTC and sys_tz have positives and negatives. And I'm not
> > sure which option is better.
>
> The one big argument for always UTC is simplicity. Always using UTC
> kills some arcane an unusual (for Linux file systems) code, and given
> how exfat implementations deal with the time zone on reading should
> always interoperate fine with other implementations.

Now I think that using UTC by default is the better option as sys_tz.
Simplicity and "no surprise" (container may use UTC, but kernel has
sys_tz in not in UTC) seems like a good arguments.

--
Pali Rohár
[email protected]

2020-01-17 04:53:39

by Namjae Jeon

[permalink] [raw]
Subject: RE: [PATCH v10 09/14] exfat: add misc operations


> This is what I think the timezone mount option should be used
> for: if we don't know what the timezone was for the on-disk timestamp, use
> the one provided by the user. However, if none was specified, it should be
> either sys_tz or UTC (i.e. no conversion). I would prefer the use of UTC
> here given the problems with sys_tz, but sys_tz would be more consistent
> with how fs/fat works.
Hi Arnd,

Could you please review this change ?

/* Convert a EXFAT time/date pair to a UNIX date (seconds since 1 1 70). */
void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
__le16 time, __le16 date, u8 tz)
{
u16 t = le16_to_cpu(time);
u16 d = le16_to_cpu(date);

ts->tv_sec = mktime64(1980 + (d >> 9), d >> 5 & 0x000F, d & 0x001F,
t >> 11, (t >> 5) & 0x003F, (t & 0x001F) << 1);
ts->tv_nsec = 0;

if (tz & EXFAT_TZ_VALID)
/* Adjust timezone to UTC0. */
exfat_adjust_tz(ts, tz & ~EXFAT_TZ_VALID);
else
/* Convert from local time to UTC using time_offset. */
ts->tv_sec -= sbi->options.time_offset * SECS_PER_MIN;
}

/* Convert linear UNIX date to a EXFAT time/date pair. */
void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
__le16 *time, __le16 *date, u8 *tz)
{
struct tm tm;
u16 t, d;

time64_to_tm(ts->tv_sec, 0, &tm);
t = (tm.tm_hour << 11) | (tm.tm_min << 5) | (tm.tm_sec >> 1);
d = ((tm.tm_year - 80) << 9) | ((tm.tm_mon + 1) << 5) | tm.tm_mday;

*time = cpu_to_le16(t);
*date = cpu_to_le16(d);

/*
* Record 00h value for OffsetFromUtc field and 1 value for OffsetValid
* to indicate that local time and UTC are the same.
*/
*tz = EXFAT_TZ_VALID;
}

Thanks!

2020-01-17 10:16:38

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

On Fri, Jan 17, 2020 at 3:59 AM Namjae Jeon <[email protected]> wrote:
>
>
> > This is what I think the timezone mount option should be used
> > for: if we don't know what the timezone was for the on-disk timestamp, use
> > the one provided by the user. However, if none was specified, it should be
> > either sys_tz or UTC (i.e. no conversion). I would prefer the use of UTC
> > here given the problems with sys_tz, but sys_tz would be more consistent
> > with how fs/fat works.
> Hi Arnd,
>
> Could you please review this change ?

Looks all good to me now.

Arnd

> /* Convert a EXFAT time/date pair to a UNIX date (seconds since 1 1 70). */
> void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
> __le16 time, __le16 date, u8 tz)
> {
> u16 t = le16_to_cpu(time);
> u16 d = le16_to_cpu(date);
>
> ts->tv_sec = mktime64(1980 + (d >> 9), d >> 5 & 0x000F, d & 0x001F,
> t >> 11, (t >> 5) & 0x003F, (t & 0x001F) << 1);
> ts->tv_nsec = 0;
>
> if (tz & EXFAT_TZ_VALID)
> /* Adjust timezone to UTC0. */
> exfat_adjust_tz(ts, tz & ~EXFAT_TZ_VALID);
> else
> /* Convert from local time to UTC using time_offset. */
> ts->tv_sec -= sbi->options.time_offset * SECS_PER_MIN;
> }
>
> /* Convert linear UNIX date to a EXFAT time/date pair. */
> void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts,
> __le16 *time, __le16 *date, u8 *tz)
> {
> struct tm tm;
> u16 t, d;
>
> time64_to_tm(ts->tv_sec, 0, &tm);
> t = (tm.tm_hour << 11) | (tm.tm_min << 5) | (tm.tm_sec >> 1);
> d = ((tm.tm_year - 80) << 9) | ((tm.tm_mon + 1) << 5) | tm.tm_mday;
>
> *time = cpu_to_le16(t);
> *date = cpu_to_le16(d);
>
> /*
> * Record 00h value for OffsetFromUtc field and 1 value for OffsetValid
> * to indicate that local time and UTC are the same.
> */
> *tz = EXFAT_TZ_VALID;
> }
>
> Thanks!
>

2020-01-17 12:14:56

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH v10 09/14] exfat: add misc operations

2020-01-17 18:13 GMT+08:00, Arnd Bergmann <[email protected]>:
> On Fri, Jan 17, 2020 at 3:59 AM Namjae Jeon <[email protected]>
> wrote:
>>
>>
>> > This is what I think the timezone mount option should be used
>> > for: if we don't know what the timezone was for the on-disk timestamp,
>> > use
>> > the one provided by the user. However, if none was specified, it should
>> > be
>> > either sys_tz or UTC (i.e. no conversion). I would prefer the use of
>> > UTC
>> > here given the problems with sys_tz, but sys_tz would be more
>> > consistent
>> > with how fs/fat works.
>> Hi Arnd,
>>
>> Could you please review this change ?
>
> Looks all good to me now.
Thanks for your review!