public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
@ 2009-07-14  8:03 Zhaolei
  2009-07-14  8:04 ` [PATCH 2/2] fs/fat: Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
                   ` (3 more replies)
  0 siblings, 4 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-14  8:03 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, Andrew Morton, OGAWA Hirofumi; +Cc: linux-kernel

There are many similar code in kernel for one object:
convert time between calendar time and broken-down time.

Here is some source I found:
fs/ncpfs/dir.c
fs/smbfs/proc.c
fs/fat/misc.c
fs/udf/udftime.c
fs/cifs/netmisc.c
net/netfilter/xt_time.c
drivers/scsi/ips.c
drivers/input/misc/hp_sdc_rtc.c
drivers/rtc/rtc-lib.c
arch/ia64/hp/sim/boot/fw-emu.c
arch/m68k/mac/misc.c
arch/powerpc/kernel/time.c
arch/parisc/include/asm/rtc.h
...

We can make a common function for this type of conversion,
At least we can get following benefit:
1: Make kernel simple and unify
2: Easy to fix bug in converting code
3: Reduce clone of code in future
   For example, I'm trying to make ftrace display walltime,
   this patch will make me easy.

This code is based on code from glibc-2.6

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 include/linux/time.h   |    9 +++
 kernel/time/Makefile   |    2 +-
 kernel/time/timeconv.c |  180 
++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 190 insertions(+), 1 deletions(-)
 create mode 100644 kernel/time/timeconv.c

diff --git a/include/linux/time.h b/include/linux/time.h
index ea16c1a..0ae0e6e 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -151,6 +151,15 @@ extern void update_xtime_cache(u64 nsec);
 struct tms;
 extern void do_sys_times(struct tms *);
 
+extern void gmtime(__kernel_time_t totalsecs,
+        unsigned int *year, unsigned int *mon, unsigned int *mday,
+        unsigned int *hour, unsigned int *min, unsigned int *sec,
+        unsigned int *wday, unsigned int *yday);
+extern void localtime(__kernel_time_t totalsecs,
+        unsigned int *year, unsigned int *mon, unsigned int *mday,
+        unsigned int *hour, unsigned int *min, unsigned int *sec,
+        unsigned int *wday, unsigned int *yday);
+
 /**
  * timespec_to_ns - Convert timespec to nanoseconds
  * @ts:        pointer to the timespec variable to be converted
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 0b0a636..ee26662 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,4 +1,4 @@
-obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o 
timecompare.o
+obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o 
timecompare.o timeconv.o
 
 obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)        += clockevents.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS)        += tick-common.o
diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
new file mode 100644
index 0000000..c710605
--- /dev/null
+++ b/kernel/time/timeconv.c
@@ -0,0 +1,180 @@
+/*
+ * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, 
Inc.
+ * This file is part of the GNU C Library.
+ * Contributed by Paul Eggert (eggert@twinsun.com).
+ *
+ * The GNU C Library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * The GNU C Library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with the GNU C Library; see the file COPYING.LIB.  If not,
+ * write to the Free Software Foundation, Inc., 59 Temple Place - Suite 
330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+/*
+ * Converts the calendar time to broken-down time representation
+ * Based on code from glibc-2.6
+ *
+ * 2009-7-14:
+ *   Moved from glibc-2.6 to kernel by Zhaolei<zhaolei@cn.fujitsu.com>
+ */
+
+#include <linux/time.h>
+#include <linux/module.h>
+
+/*
+ * Nonzero if YEAR is a leap year (every 4 years,
+ * except every 100th isn't, and every 400th is).
+ */
+static inline int __isleap(unsigned int year)
+{
+    return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
+}
+
+/* How many days come before each month (0-12). */
+static const unsigned short __mon_yday[2][13] =
+  {
+    /* Normal years. */
+    {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
+    /* Leap years. */
+    {0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
+  };
+
+#define SECS_PER_HOUR    (60 * 60)
+#define SECS_PER_DAY    (SECS_PER_HOUR * 24)
+
+static void __offtime(__kernel_time_t totalsecs, int offset,
+         unsigned int *year, unsigned int *mon, unsigned int *mday,
+         unsigned int *hour, unsigned int *min, unsigned int *sec,
+         unsigned int *wday, unsigned int *yday)
+{
+    long days, rem, y;
+    const unsigned short *ip;
+
+    days = totalsecs / SECS_PER_DAY;
+    rem = totalsecs % SECS_PER_DAY;
+    rem += offset;
+    while (rem < 0) {
+        rem += SECS_PER_DAY;
+        --days;
+    }
+    while (rem >= SECS_PER_DAY) {
+        rem -= SECS_PER_DAY;
+        ++days;
+    }
+
+    if (hour)
+        *hour = rem / SECS_PER_HOUR;
+    rem %= SECS_PER_HOUR;
+    if (min)
+        *min = rem / 60;
+    if (sec)
+        *sec = rem % 60;
+
+    if (wday) {
+        /* January 1, 1970 was a Thursday. */
+        *wday = (4 + days) % 7;
+        if (*wday < 0)
+            *wday += 7;
+    }
+
+    y = 1970;
+
+#define DIV(a, b) ((a) / (b) - ((a) % (b) < 0))
+#define LEAPS_THRU_END_OF(y) (DIV(y, 4) - DIV(y, 100) + DIV(y, 400))
+
+    while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
+        /* Guess a corrected year, assuming 365 days per year. */
+        long yg = y + days / 365 - (days % 365 < 0);
+
+        /* Adjust DAYS and Y to match the guessed year. */
+        days -= ((yg - y) * 365
+             + LEAPS_THRU_END_OF(yg - 1)
+             - LEAPS_THRU_END_OF(y - 1));
+        y = yg;
+    }
+    if (year) {
+        *year = y - 1900;
+        if (*year != y - 1900) {
+            /* The year cannot be represented due to overflow. */
+            *year = -1;
+        }
+    }
+
+    if (yday)
+        *yday = days;
+
+    ip = __mon_yday[__isleap(y)];
+    for (y = 11; days < ip[y]; y--)
+        continue;
+    days -= ip[y];
+    if (mon)
+        *mon = y;
+    if (mday)
+        *mday = days + 1;
+}
+
+/**
+ * gmtime - converts the calendar time to UTC broken-down time
+ *
+ * @totalsecs    The number of seconds elapsed since 00:00:00 on 
January 1, 1970,
+ *        Coordinated Universal Time (UTC).
+ * @year    Store the number of years since 1900.
+ * @mon        Store the number of months since January, in the range 0 
to 11.
+ * @mday    Store the day of the month, in the range 1 to 31.
+ * @hour    Store the number of hours past midnight, in the range 0 to 23.
+ * @min        Store the number of minutes after the hour,
+ *        in the range 0 to 59.
+ * @sec        Store the number of seconds after the minute, normally 
in the
+ *        range 0 to 59, but can be up to 60 to allow for leap seconds.
+ * @wday    Store the number of days since Sunday, in the range 0 to 6.
+ * @yday    Store the number of days since January 1, in the range 0 to 
365.
+ *
+ * Similar to the gmtime() in glibc, broken-down time is expressed in
+ * Coordinated Universal Time (UTC).
+ */
+void gmtime(__kernel_time_t totalsecs,
+         unsigned int *year, unsigned int *mon, unsigned int *mday,
+         unsigned int *hour, unsigned int *min, unsigned int *sec,
+         unsigned int *wday, unsigned int *yday)
+{
+    __offtime(totalsecs, 0, year, mon, mday, hour, min, sec, wday, yday);
+}
+EXPORT_SYMBOL(gmtime);
+
+/**
+ * localtime - converts the calendar time to local broken-down time
+ *
+ * @totalsecs    The number of seconds elapsed since 00:00:00 on 
January 1, 1970,
+ *        Coordinated Universal Time (UTC).
+ * @year    Store the number of years since 1900.
+ * @mon        Store the number of months since January, in the range 0 
to 11.
+ * @mday    Store the day of the month, in the range 1 to 31.
+ * @hour    Store the number of hours past midnight, in the range 0 to 23.
+ * @min        Store the number of minutes after the hour,
+ *        in the range 0 to 59.
+ * @sec        Store the number of seconds after the minute, normally 
in the
+ *        range 0 to 59, but can be up to 60 to allow for leap seconds.
+ * @wday    Store the number of days since Sunday, in the range 0 to 6.
+ * @yday    Store the number of days since January 1, in the range 0 to 
365.
+ *
+ * Similar to the localtime() in glibc, broken-down time is expressed
+ * relative to sys_tz.
+ */
+void localtime(__kernel_time_t totalsecs,
+         unsigned int *year, unsigned int *mon, unsigned int *mday,
+         unsigned int *hour, unsigned int *min, unsigned int *sec,
+         unsigned int *wday, unsigned int *yday)
+{
+    __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, year, mon, mday, 
hour,
+        min, sec, wday, yday);
+}
+EXPORT_SYMBOL(localtime);
-- 
1.5.5.3



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 2/2] fs/fat: Use common localtime/gmtime in fat_time_unix2fat()
  2009-07-14  8:03 [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Zhaolei
@ 2009-07-14  8:04 ` Zhaolei
  2009-07-14 22:10 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Andrew Morton
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-14  8:04 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, Andrew Morton, OGAWA Hirofumi; +Cc: linux-kernel

It is not necessary to write custom code for convert calendar time
to broken-down time.
localtime()/gmtime() is more generic to do that.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 fs/fat/misc.c |   62 
++++++++++++++++++--------------------------------------
 1 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index a6c2047..d9b6552 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/buffer_head.h>
+#include <linux/time.h>
 #include "fat.h"
 
 /*
@@ -155,10 +156,6 @@ extern struct timezone sys_tz;
 #define SECS_PER_MIN    60
 #define SECS_PER_HOUR    (60 * 60)
 #define SECS_PER_DAY    (SECS_PER_HOUR * 24)
-#define UNIX_SECS_1980    315532800L
-#if BITS_PER_LONG == 64
-#define UNIX_SECS_2108    4354819200L
-#endif
 /* days between 1.1.70 and 1.1.80 (2 leap days) */
 #define DAYS_DELTA    (365 * 10 + 2)
 /* 120 (2100 - 1980) isn't leap year */
@@ -211,58 +208,40 @@ void fat_time_fat2unix(struct msdos_sb_info *sbi, 
struct timespec *ts,
 void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
                __le16 *time, __le16 *date, u8 *time_cs)
 {
-    time_t second = ts->tv_sec;
-    time_t day, leap_day, month, year;
-
-    if (!sbi->options.tz_utc)
-        second -= sys_tz.tz_minuteswest * SECS_PER_MIN;
+    unsigned int year, mon, mday, hour, min, sec;
+    if (sbi->options.tz_utc) {
+        gmtime(ts->tv_sec,
+               &year, &mon, &mday, &hour, &min, &sec, NULL, NULL);
+    } else {
+        localtime(ts->tv_sec,
+               &year, &mon, &mday, &hour, &min, &sec, NULL, NULL);
+    }
 
-    /* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
-    if (second < UNIX_SECS_1980) {
+    /*  FAT can only support year between 1980 to 2107 */
+    if (year < 1980 - 1900) {
         *time = 0;
         *date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
         if (time_cs)
             *time_cs = 0;
         return;
     }
-#if BITS_PER_LONG == 64
-    if (second >= UNIX_SECS_2108) {
+    if (year > 2107 - 1900) {
         *time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
         *date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
         if (time_cs)
             *time_cs = 199;
         return;
     }
-#endif
 
-    day = second / SECS_PER_DAY - DAYS_DELTA;
-    year = day / 365;
-    leap_day = (year + 3) / 4;
-    if (year > YEAR_2100)        /* 2100 isn't leap year */
-        leap_day--;
-    if (year * 365 + leap_day > day)
-        year--;
-    leap_day = (year + 3) / 4;
-    if (year > YEAR_2100)        /* 2100 isn't leap year */
-        leap_day--;
-    day -= year * 365 + leap_day;
+    /* from 1900 -> from 1980 */
+    year -= 80;
+    /* 0~11 -> 1~12 */
+    mon++;
+    /* 0~59 -> 0~29(2sec counts) */
+    sec >>= 1;
 
-    if (IS_LEAP_YEAR(year) && day == days_in_year[3]) {
-        month = 2;
-    } else {
-        if (IS_LEAP_YEAR(year) && day > days_in_year[3])
-            day--;
-        for (month = 1; month < 12; month++) {
-            if (days_in_year[month + 1] > day)
-                break;
-        }
-    }
-    day -= days_in_year[month];
-
-    *time = cpu_to_le16(((second / SECS_PER_HOUR) % 24) << 11
-                | ((second / SECS_PER_MIN) % 60) << 5
-                | (second % SECS_PER_MIN) >> 1);
-    *date = cpu_to_le16((year << 9) | (month << 5) | (day + 1));
+    *time = cpu_to_le16(hour << 11 | min << 5 | sec);
+    *date = cpu_to_le16(year << 9 | mon << 5 | mday);
     if (time_cs)
         *time_cs = (ts->tv_sec & 1) * 100 + ts->tv_nsec / 10000000;
 }
@@ -283,4 +262,3 @@ int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
     }
     return err;
 }
-
-- 
1.5.5.3



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-14  8:03 [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Zhaolei
  2009-07-14  8:04 ` [PATCH 2/2] fs/fat: Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
@ 2009-07-14 22:10 ` Andrew Morton
  2009-07-15  0:59   ` Zhaolei
                     ` (2 more replies)
  2009-07-18 10:02 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Ingo Molnar
  2009-07-18 12:26 ` Andi Kleen
  3 siblings, 3 replies; 33+ messages in thread
From: Andrew Morton @ 2009-07-14 22:10 UTC (permalink / raw)
  To: Zhaolei; +Cc: mingo, tglx, hirofumi, linux-kernel

On Tue, 14 Jul 2009 16:03:12 +0800
Zhaolei <zhaolei@cn.fujitsu.com> wrote:

> There are many similar code in kernel for one object:
> convert time between calendar time and broken-down time.
> 
> Here is some source I found:
> fs/ncpfs/dir.c
> fs/smbfs/proc.c
> fs/fat/misc.c
> fs/udf/udftime.c
> fs/cifs/netmisc.c
> net/netfilter/xt_time.c
> drivers/scsi/ips.c
> drivers/input/misc/hp_sdc_rtc.c
> drivers/rtc/rtc-lib.c
> arch/ia64/hp/sim/boot/fw-emu.c
> arch/m68k/mac/misc.c
> arch/powerpc/kernel/time.c
> arch/parisc/include/asm/rtc.h
> ...
> 
> We can make a common function for this type of conversion,
> At least we can get following benefit:
> 1: Make kernel simple and unify
> 2: Easy to fix bug in converting code
> 3: Reduce clone of code in future
>    For example, I'm trying to make ftrace display walltime,
>    this patch will make me easy.
> 

The objective is a good one.  Have you verified that these new library
functions can be used by most/all of the above callers?

> 
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> ---
>  include/linux/time.h   |    9 +++
>  kernel/time/Makefile   |    2 +-
>  kernel/time/timeconv.c |  180 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 190 insertions(+), 1 deletions(-)
>  create mode 100644 kernel/time/timeconv.c
> 
> diff --git a/include/linux/time.h b/include/linux/time.h
> index ea16c1a..0ae0e6e 100644
> --- a/include/linux/time.h
> +++ b/include/linux/time.h
> @@ -151,6 +151,15 @@ extern void update_xtime_cache(u64 nsec);
>  struct tms;
>  extern void do_sys_times(struct tms *);
>  
> +extern void gmtime(__kernel_time_t totalsecs,
> +        unsigned int *year, unsigned int *mon, unsigned int *mday,
> +        unsigned int *hour, unsigned int *min, unsigned int *sec,
> +        unsigned int *wday, unsigned int *yday);
> +extern void localtime(__kernel_time_t totalsecs,
> +        unsigned int *year, unsigned int *mon, unsigned int *mday,
> +        unsigned int *hour, unsigned int *min, unsigned int *sec,
> +        unsigned int *wday, unsigned int *yday);

I'd imagine that many callers would at least need some types changed -
replace `int' with `unsigned int', etc.  Not a big problem.

>  /**
>   * timespec_to_ns - Convert timespec to nanoseconds
>   * @ts:        pointer to the timespec variable to be converted
> diff --git a/kernel/time/Makefile b/kernel/time/Makefile
> index 0b0a636..ee26662 100644
> --- a/kernel/time/Makefile
> +++ b/kernel/time/Makefile
> @@ -1,4 +1,4 @@
> -obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o 
> timecompare.o
> +obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o 
> timecompare.o timeconv.o
>  
>  obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)        += clockevents.o
>  obj-$(CONFIG_GENERIC_CLOCKEVENTS)        += tick-common.o
> diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
> new file mode 100644
> index 0000000..c710605
> --- /dev/null
> +++ b/kernel/time/timeconv.c
> @@ -0,0 +1,180 @@
> +/*
> + * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, 
> Inc.

The patch is significantly wordwrapped by your email client.

The patch has no tabs in it at all - either some serious cleanup is
needed of your email client is performing tab-to-space conversion.

> + * This file is part of the GNU C Library.
> + * Contributed by Paul Eggert (eggert@twinsun.com).
> + *
> + * The GNU C Library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Library General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * The GNU C Library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Library General Public License for more details.
> + *
> + * You should have received a copy of the GNU Library General Public
> + * License along with the GNU C Library; see the file COPYING.LIB.  If not,
> + * write to the Free Software Foundation, Inc., 59 Temple Place - Suite 
> 330,
> + * Boston, MA 02111-1307, USA.
> + */
> +
> +/*
> + * Converts the calendar time to broken-down time representation
> + * Based on code from glibc-2.6
> + *
> + * 2009-7-14:
> + *   Moved from glibc-2.6 to kernel by Zhaolei<zhaolei@cn.fujitsu.com>
> + */
> +
> +#include <linux/time.h>
> +#include <linux/module.h>
> +
> +/*
> + * Nonzero if YEAR is a leap year (every 4 years,
> + * except every 100th isn't, and every 400th is).
> + */
> +static inline int __isleap(unsigned int year)
> +{
> +    return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
> +}

The explicit `inline' probably isn't beneficial here.

> +/* How many days come before each month (0-12). */
> +static const unsigned short __mon_yday[2][13] =
> +  {
> +    /* Normal years. */
> +    {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
> +    /* Leap years. */
> +    {0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
> +  };
> +
> +#define SECS_PER_HOUR    (60 * 60)
> +#define SECS_PER_DAY    (SECS_PER_HOUR * 24)

I wonder if these whould be in a header.  I guess not, if there aren't
any sites which can use this.

> +static void __offtime(__kernel_time_t totalsecs, int offset,
> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
> +         unsigned int *wday, unsigned int *yday)
> +{
> +    long days, rem, y;
> +    const unsigned short *ip;
> +
> +    days = totalsecs / SECS_PER_DAY;
> +    rem = totalsecs % SECS_PER_DAY;
> +    rem += offset;
> +    while (rem < 0) {
> +        rem += SECS_PER_DAY;
> +        --days;
> +    }
> +    while (rem >= SECS_PER_DAY) {
> +        rem -= SECS_PER_DAY;
> +        ++days;
> +    }
> +
> +    if (hour)
> +        *hour = rem / SECS_PER_HOUR;
> +    rem %= SECS_PER_HOUR;
> +    if (min)
> +        *min = rem / 60;
> +    if (sec)
> +        *sec = rem % 60;
> +
> +    if (wday) {
> +        /* January 1, 1970 was a Thursday. */
> +        *wday = (4 + days) % 7;
> +        if (*wday < 0)
> +            *wday += 7;
> +    }

The code should all be converted to standard kernel style, please. 
Mainly the use of hard tabs.


> +    y = 1970;
> +
> +#define DIV(a, b) ((a) / (b) - ((a) % (b) < 0))

hm, I wonder what that does.

It would be clearer to convert this into a regular C function, along
with a comment whcih explains what it does.

> +#define LEAPS_THRU_END_OF(y) (DIV(y, 4) - DIV(y, 100) + DIV(y, 400))

Ditto.

> +    while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
> +        /* Guess a corrected year, assuming 365 days per year. */
> +        long yg = y + days / 365 - (days % 365 < 0);
> +
> +        /* Adjust DAYS and Y to match the guessed year. */
> +        days -= ((yg - y) * 365
> +             + LEAPS_THRU_END_OF(yg - 1)
> +             - LEAPS_THRU_END_OF(y - 1));
> +        y = yg;
> +    }
> +    if (year) {
> +        *year = y - 1900;
> +        if (*year != y - 1900) {
> +            /* The year cannot be represented due to overflow. */
> +            *year = -1;
> +        }
> +    }
> +
> +    if (yday)
> +        *yday = days;
> +
> +    ip = __mon_yday[__isleap(y)];
> +    for (y = 11; days < ip[y]; y--)
> +        continue;
> +    days -= ip[y];
> +    if (mon)
> +        *mon = y;
> +    if (mday)
> +        *mday = days + 1;
> +}

>
> ...
>
> +void gmtime(__kernel_time_t totalsecs,
> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
> +         unsigned int *wday, unsigned int *yday)
> +{
> +    __offtime(totalsecs, 0, year, mon, mday, hour, min, sec, wday, yday);
> +}
> +EXPORT_SYMBOL(gmtime);
>
> ...
>
> +void localtime(__kernel_time_t totalsecs,
> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
> +         unsigned int *wday, unsigned int *yday)
> +{
> +    __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, year, mon, mday, 
> hour,
> +        min, sec, wday, yday);
> +}
> +EXPORT_SYMBOL(localtime);

These are such simple wrappers around __offtime() that it might be
better to make them static inlines in time.h, so callers will end up
directly calling __offtime() rather than having to remarshal ten
function arguments.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-14 22:10 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Andrew Morton
@ 2009-07-15  0:59   ` Zhaolei
  2009-07-15  7:23   ` [PATCH v2 0/2] " Zhaolei
  2009-07-18 11:50   ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Pavel Machek
  2 siblings, 0 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-15  0:59 UTC (permalink / raw)
  To: Andrew Morton; +Cc: mingo, tglx, hirofumi, linux-kernel

Andrew Morton wrote:
> On Tue, 14 Jul 2009 16:03:12 +0800
> Zhaolei <zhaolei@cn.fujitsu.com> wrote:
>
>   
>> There are many similar code in kernel for one object:
>> convert time between calendar time and broken-down time.
>>
>> Here is some source I found:
>> fs/ncpfs/dir.c
>> fs/smbfs/proc.c
>> fs/fat/misc.c
>> fs/udf/udftime.c
>> fs/cifs/netmisc.c
>> net/netfilter/xt_time.c
>> drivers/scsi/ips.c
>> drivers/input/misc/hp_sdc_rtc.c
>> drivers/rtc/rtc-lib.c
>> arch/ia64/hp/sim/boot/fw-emu.c
>> arch/m68k/mac/misc.c
>> arch/powerpc/kernel/time.c
>> arch/parisc/include/asm/rtc.h
>> ...
>>
>> We can make a common function for this type of conversion,
>> At least we can get following benefit:
>> 1: Make kernel simple and unify
>> 2: Easy to fix bug in converting code
>> 3: Reduce clone of code in future
>>    For example, I'm trying to make ftrace display walltime,
>>    this patch will make me easy.
>>
>>     
>
> The objective is a good one.  Have you verified that these new library
> functions can be used by most/all of the above callers?
>   

Hello, Andrew

Thanks for your review.

I checked some of them, and I think this new library can make them simple.
A example is my patch for fs/fat/misc.c.

>   
>> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
>> ---
>>  include/linux/time.h   |    9 +++
>>  kernel/time/Makefile   |    2 +-
>>  kernel/time/timeconv.c |  180 
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 190 insertions(+), 1 deletions(-)
>>  create mode 100644 kernel/time/timeconv.c
>>
>> diff --git a/include/linux/time.h b/include/linux/time.h
>> index ea16c1a..0ae0e6e 100644
>> --- a/include/linux/time.h
>> +++ b/include/linux/time.h
>> @@ -151,6 +151,15 @@ extern void update_xtime_cache(u64 nsec);
>>  struct tms;
>>  extern void do_sys_times(struct tms *);
>>  
>> +extern void gmtime(__kernel_time_t totalsecs,
>> +        unsigned int *year, unsigned int *mon, unsigned int *mday,
>> +        unsigned int *hour, unsigned int *min, unsigned int *sec,
>> +        unsigned int *wday, unsigned int *yday);
>> +extern void localtime(__kernel_time_t totalsecs,
>> +        unsigned int *year, unsigned int *mon, unsigned int *mday,
>> +        unsigned int *hour, unsigned int *min, unsigned int *sec,
>> +        unsigned int *wday, unsigned int *yday);
>>     
>
> I'd imagine that many callers would at least need some types changed -
> replace `int' with `unsigned int', etc.  Not a big problem.
>   

Actually, I considered to use int instead, but at last I use unsigned int to
make it same with mktime() which is already in kernel.

>   
>>  /**
>>   * timespec_to_ns - Convert timespec to nanoseconds
>>   * @ts:        pointer to the timespec variable to be converted
>> diff --git a/kernel/time/Makefile b/kernel/time/Makefile
>> index 0b0a636..ee26662 100644
>> --- a/kernel/time/Makefile
>> +++ b/kernel/time/Makefile
>> @@ -1,4 +1,4 @@
>> -obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o 
>> timecompare.o
>> +obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o 
>> timecompare.o timeconv.o
>>  
>>  obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)        += clockevents.o
>>  obj-$(CONFIG_GENERIC_CLOCKEVENTS)        += tick-common.o
>> diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
>> new file mode 100644
>> index 0000000..c710605
>> --- /dev/null
>> +++ b/kernel/time/timeconv.c
>> @@ -0,0 +1,180 @@
>> +/*
>> + * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, 
>> Inc.
>>     
>
> The patch is significantly wordwrapped by your email client.
>
> The patch has no tabs in it at all - either some serious cleanup is
> needed of your email client is performing tab-to-space conversion.
>   

Sorry...I reinstalled a email client......
I'll fix it and resend this patch.


>> + * This file is part of the GNU C Library.
>> + * Contributed by Paul Eggert (eggert@twinsun.com).
>> + *
>> + * The GNU C Library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Library General Public License as
>> + * published by the Free Software Foundation; either version 2 of the
>> + * License, or (at your option) any later version.
>> + *
>> + * The GNU C Library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Library General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Library General Public
>> + * License along with the GNU C Library; see the file COPYING.LIB.  If not,
>> + * write to the Free Software Foundation, Inc., 59 Temple Place - Suite 
>> 330,
>> + * Boston, MA 02111-1307, USA.
>> + */
>> +
>> +/*
>> + * Converts the calendar time to broken-down time representation
>> + * Based on code from glibc-2.6
>> + *
>> + * 2009-7-14:
>> + *   Moved from glibc-2.6 to kernel by Zhaolei<zhaolei@cn.fujitsu.com>
>> + */
>> +
>> +#include <linux/time.h>
>> +#include <linux/module.h>
>> +
>> +/*
>> + * Nonzero if YEAR is a leap year (every 4 years,
>> + * except every 100th isn't, and every 400th is).
>> + */
>> +static inline int __isleap(unsigned int year)
>> +{
>> +    return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
>> +}
>>     
>
> The explicit `inline' probably isn't beneficial here.
>   

OK, I'll remove it.

>   
>> +/* How many days come before each month (0-12). */
>> +static const unsigned short __mon_yday[2][13] =
>> +  {
>> +    /* Normal years. */
>> +    {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
>> +    /* Leap years. */
>> +    {0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
>> +  };
>> +
>> +#define SECS_PER_HOUR    (60 * 60)
>> +#define SECS_PER_DAY    (SECS_PER_HOUR * 24)
>>     
>
> I wonder if these whould be in a header.  I guess not, if there aren't
> any sites which can use this.
>   

Agree, IMHO, it is not necessary to move it into header.

>   
>> +static void __offtime(__kernel_time_t totalsecs, int offset,
>> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
>> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
>> +         unsigned int *wday, unsigned int *yday)
>> +{
>> +    long days, rem, y;
>> +    const unsigned short *ip;
>> +
>> +    days = totalsecs / SECS_PER_DAY;
>> +    rem = totalsecs % SECS_PER_DAY;
>> +    rem += offset;
>> +    while (rem < 0) {
>> +        rem += SECS_PER_DAY;
>> +        --days;
>> +    }
>> +    while (rem >= SECS_PER_DAY) {
>> +        rem -= SECS_PER_DAY;
>> +        ++days;
>> +    }
>> +
>> +    if (hour)
>> +        *hour = rem / SECS_PER_HOUR;
>> +    rem %= SECS_PER_HOUR;
>> +    if (min)
>> +        *min = rem / 60;
>> +    if (sec)
>> +        *sec = rem % 60;
>> +
>> +    if (wday) {
>> +        /* January 1, 1970 was a Thursday. */
>> +        *wday = (4 + days) % 7;
>> +        if (*wday < 0)
>> +            *wday += 7;
>> +    }
>>     
>
> The code should all be converted to standard kernel style, please. 
> Mainly the use of hard tabs.
>   

Sorry, I'll resend this patch.

>> +    y = 1970;
>> +
>> +#define DIV(a, b) ((a) / (b) - ((a) % (b) < 0))
>>     
>
> hm, I wonder what that does.
>
> It would be clearer to convert this into a regular C function, along
> with a comment whcih explains what it does.
>
>   
>> +#define LEAPS_THRU_END_OF(y) (DIV(y, 4) - DIV(y, 100) + DIV(y, 400))
>>     
>
> Ditto.
>   

It is because I try to keep similar style with code in glibc so that updates
in glibc can easy to applied here.
But actually it looks ugly, I'll fix it.


>   
>> +    while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
>> +        /* Guess a corrected year, assuming 365 days per year. */
>> +        long yg = y + days / 365 - (days % 365 < 0);
>> +
>> +        /* Adjust DAYS and Y to match the guessed year. */
>> +        days -= ((yg - y) * 365
>> +             + LEAPS_THRU_END_OF(yg - 1)
>> +             - LEAPS_THRU_END_OF(y - 1));
>> +        y = yg;
>> +    }
>> +    if (year) {
>> +        *year = y - 1900;
>> +        if (*year != y - 1900) {
>> +            /* The year cannot be represented due to overflow. */
>> +            *year = -1;
>> +        }
>> +    }
>> +
>> +    if (yday)
>> +        *yday = days;
>> +
>> +    ip = __mon_yday[__isleap(y)];
>> +    for (y = 11; days < ip[y]; y--)
>> +        continue;
>> +    days -= ip[y];
>> +    if (mon)
>> +        *mon = y;
>> +    if (mday)
>> +        *mday = days + 1;
>> +}
>>     
>
>   
>> ...
>>
>> +void gmtime(__kernel_time_t totalsecs,
>> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
>> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
>> +         unsigned int *wday, unsigned int *yday)
>> +{
>> +    __offtime(totalsecs, 0, year, mon, mday, hour, min, sec, wday, yday);
>> +}
>> +EXPORT_SYMBOL(gmtime);
>>
>> ...
>>
>> +void localtime(__kernel_time_t totalsecs,
>> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
>> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
>> +         unsigned int *wday, unsigned int *yday)
>> +{
>> +    __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, year, mon, mday, 
>> hour,
>> +        min, sec, wday, yday);
>> +}
>> +EXPORT_SYMBOL(localtime);
>>     
>
> These are such simple wrappers around __offtime() that it might be
> better to make them static inlines in time.h, so callers will end up
> directly calling __offtime() rather than having to remarshal ten
> function arguments.
>   

Good idea, will fix.

Thanks
Zhaolei



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 0/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-14 22:10 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Andrew Morton
  2009-07-15  0:59   ` Zhaolei
@ 2009-07-15  7:23   ` Zhaolei
  2009-07-15  7:24     ` [PATCH v2 1/2] " Zhaolei
  2009-07-15  7:25     ` [PATCH v2 2/2] Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
  2009-07-18 11:50   ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Pavel Machek
  2 siblings, 2 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-15  7:23 UTC (permalink / raw)
  To: Andrew Morton; +Cc: mingo, tglx, hirofumi, linux-kernel

Hello, Andrew

I modified this patch based on your review.
Here is changes:
1: Fix "no tabs" error caused by email client.
2: Remove explicit 'inline' in __isleap()
3: Move DIV() and LEAPS_THRU_END_OF(y) into regular C function along with
   comment
4: Move gmtime() and localtime() into header file.

Thanks
Zhaolei


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-15  7:23   ` [PATCH v2 0/2] " Zhaolei
@ 2009-07-15  7:24     ` Zhaolei
  2009-07-15  7:25     ` [PATCH v2 2/2] Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
  1 sibling, 0 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-15  7:24 UTC (permalink / raw)
  To: Andrew Morton; +Cc: mingo, tglx, hirofumi, linux-kernel

There are many similar code in kernel for one object:
convert time between calendar time and broken-down time.

Here is some source I found:
fs/ncpfs/dir.c
fs/smbfs/proc.c
fs/fat/misc.c
fs/udf/udftime.c
fs/cifs/netmisc.c
net/netfilter/xt_time.c
drivers/scsi/ips.c
drivers/input/misc/hp_sdc_rtc.c
drivers/rtc/rtc-lib.c
arch/ia64/hp/sim/boot/fw-emu.c
arch/m68k/mac/misc.c
arch/powerpc/kernel/time.c
arch/parisc/include/asm/rtc.h
...

We can make a common function for this type of convert,
At least we can get following benefit:
1: Make kernel simple and unify
2: Easy to fix bug in converting code
3: Reduce clone of code in future
   For example, I'm trying to make ftrace display walltime,
   this patch will make me easy.

This code is based on code from glibc-2.6

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 include/linux/time.h   |   62 +++++++++++++++++++
 kernel/time/Makefile   |    2 +-
 kernel/time/timeconv.c |  153 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 216 insertions(+), 1 deletions(-)
 create mode 100644 kernel/time/timeconv.c

diff --git a/include/linux/time.h b/include/linux/time.h
index ea16c1a..658f8e1 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -151,6 +151,68 @@ extern void update_xtime_cache(u64 nsec);
 struct tms;
 extern void do_sys_times(struct tms *);
 
+extern void __offtime(__kernel_time_t totalsecs, int offset,
+		      unsigned int *year, unsigned int *mon, unsigned int *mday,
+		      unsigned int *hour, unsigned int *min, unsigned int *sec,
+		      unsigned int *wday, unsigned int *yday);
+
+/**
+ * gmtime - converts the calendar time to UTC broken-down time
+ *
+ * @totalsecs	The number of seconds elapsed since 00:00:00 on January 1, 1970,
+ *		Coordinated Universal Time (UTC).
+ * @year	Store the number of years since 1900.
+ * @mon		Store the number of months since January, in the range 0 to 11.
+ * @mday	Store the day of the month, in the range 1 to 31.
+ * @hour	Store the number of hours past midnight, in the range 0 to 23.
+ * @min		Store the number of minutes after the hour,
+ *		in the range 0 to 59.
+ * @sec		Store the number of seconds after the minute, normally in the
+ *		range 0 to 59, but can be up to 60 to allow for leap seconds.
+ * @wday	Store the number of days since Sunday, in the range 0 to 6.
+ * @yday	Store the number of days since January 1, in the range 0 to 365.
+ *
+ * Similar to the gmtime() in glibc, broken-down time is expressed in
+ * Coordinated Universal Time (UTC).
+ */
+static inline void gmtime(__kernel_time_t totalsecs,
+			  unsigned int *year, unsigned int *mon,
+			  unsigned int *mday, unsigned int *hour,
+			  unsigned int *min,  unsigned int *sec,
+			  unsigned int *wday, unsigned int *yday)
+{
+	__offtime(totalsecs, 0, year, mon, mday, hour, min, sec, wday, yday);
+}
+
+/**
+ * localtime - converts the calendar time to local broken-down time
+ *
+ * @totalsecs	The number of seconds elapsed since 00:00:00 on January 1, 1970,
+ *		Coordinated Universal Time (UTC).
+ * @year	Store the number of years since 1900.
+ * @mon		Store the number of months since January, in the range 0 to 11.
+ * @mday	Store the day of the month, in the range 1 to 31.
+ * @hour	Store the number of hours past midnight, in the range 0 to 23.
+ * @min		Store the number of minutes after the hour,
+ *		in the range 0 to 59.
+ * @sec		Store the number of seconds after the minute, normally in the
+ *		range 0 to 59, but can be up to 60 to allow for leap seconds.
+ * @wday	Store the number of days since Sunday, in the range 0 to 6.
+ * @yday	Store the number of days since January 1, in the range 0 to 365.
+ *
+ * Similar to the localtime() in glibc, broken-down time is expressed
+ * relative to sys_tz.
+ */
+static inline void localtime(__kernel_time_t totalsecs,
+			     unsigned int *year, unsigned int *mon,
+			     unsigned int *mday, unsigned int *hour,
+			     unsigned int *min,  unsigned int *sec,
+			     unsigned int *wday, unsigned int *yday)
+{
+	__offtime(totalsecs, -sys_tz.tz_minuteswest * 60, year, mon, mday, hour,
+		  min, sec, wday, yday);
+}
+
 /**
  * timespec_to_ns - Convert timespec to nanoseconds
  * @ts:		pointer to the timespec variable to be converted
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 0b0a636..ee26662 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,4 +1,4 @@
-obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o
+obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o timeconv.o
 
 obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)		+= clockevents.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS)		+= tick-common.o
diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
new file mode 100644
index 0000000..b28fcec
--- /dev/null
+++ b/kernel/time/timeconv.c
@@ -0,0 +1,153 @@
+/*
+ * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, Inc.
+ * This file is part of the GNU C Library.
+ * Contributed by Paul Eggert (eggert@twinsun.com).
+ *
+ * The GNU C Library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * The GNU C Library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with the GNU C Library; see the file COPYING.LIB.  If not,
+ * write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+/*
+ * Converts the calendar time to broken-down time representation
+ * Based on code from glibc-2.6
+ *
+ * 2009-7-14:
+ *   Moved from glibc-2.6 to kernel by Zhaolei<zhaolei@cn.fujitsu.com>
+ */
+
+#include <linux/time.h>
+#include <linux/module.h>
+
+/*
+ * Nonzero if YEAR is a leap year (every 4 years,
+ * except every 100th isn't, and every 400th is).
+ */
+static int __isleap(unsigned int year)
+{
+	return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
+}
+
+/* do a mathdiv for long type */
+static long math_div(long a, long b)
+{
+	return a / b - (a % b < 0);
+}
+
+/* How many leap years between y1 and y2, y1 must less or equal to y2 */
+static long leaps_between(long y1, long y2)
+{
+	long leaps1 = math_div(y1 - 1, 4) - math_div(y1 - 1, 100)
+		+ math_div(y1 - 1, 400);
+	long leaps2 = math_div(y2 - 1, 4) - math_div(y2 - 1, 100)
+		+ math_div(y2 - 1, 400);
+	return leaps2 - leaps1;
+}
+
+/* How many days come before each month (0-12). */
+static const unsigned short __mon_yday[2][13] = {
+	/* Normal years. */
+	{0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
+	/* Leap years. */
+	{0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
+};
+
+#define SECS_PER_HOUR	(60 * 60)
+#define SECS_PER_DAY	(SECS_PER_HOUR * 24)
+
+/**
+ * __offtime - converts the calendar time to local broken-down time
+ *
+ * @totalsecs	The number of seconds elapsed since 00:00:00 on January 1, 1970,
+ *		Coordinated Universal Time (UTC).
+ * @offset	Offset seconds adding to totalsecs.
+ * @year	Store the number of years since 1900.
+ * @mon		Store the number of months since January, in the range 0 to 11.
+ * @mday	Store the day of the month, in the range 1 to 31.
+ * @hour	Store the number of hours past midnight, in the range 0 to 23.
+ * @min		Store the number of minutes after the hour,
+ *		in the range 0 to 59.
+ * @sec		Store the number of seconds after the minute, normally in the
+ *		range 0 to 59, but can be up to 60 to allow for leap seconds.
+ * @wday	Store the number of days since Sunday, in the range 0 to 6.
+ * @yday	Store the number of days since January 1, in the range 0 to 365.
+ *
+ * This function is for internal use, call gmtime() and localtime() instead.
+ */
+void __offtime(__kernel_time_t totalsecs, int offset,
+	       unsigned int *year, unsigned int *mon, unsigned int *mday,
+	       unsigned int *hour, unsigned int *min, unsigned int *sec,
+	       unsigned int *wday, unsigned int *yday)
+{
+	long days, rem, y;
+	const unsigned short *ip;
+
+	days = totalsecs / SECS_PER_DAY;
+	rem = totalsecs % SECS_PER_DAY;
+	rem += offset;
+	while (rem < 0) {
+		rem += SECS_PER_DAY;
+		--days;
+	}
+	while (rem >= SECS_PER_DAY) {
+		rem -= SECS_PER_DAY;
+		++days;
+	}
+
+	if (hour)
+		*hour = rem / SECS_PER_HOUR;
+	rem %= SECS_PER_HOUR;
+	if (min)
+		*min = rem / 60;
+	if (sec)
+		*sec = rem % 60;
+
+	if (wday) {
+		/* January 1, 1970 was a Thursday. */
+		*wday = (4 + days) % 7;
+		if (*wday < 0)
+			*wday += 7;
+	}
+
+	y = 1970;
+
+	while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
+		/* Guess a corrected year, assuming 365 days per year. */
+		long yg = y + math_div(days, 365);
+
+		/* Adjust DAYS and Y to match the guessed year. */
+		days -= (yg - y) * 365 + leaps_between(y, yg);
+		y = yg;
+	}
+	if (year) {
+		*year = y - 1900;
+		if (*year != y - 1900) {
+			/* The year cannot be represented due to overflow. */
+			*year = -1;
+		}
+	}
+
+	if (yday)
+		*yday = days;
+
+	ip = __mon_yday[__isleap(y)];
+	for (y = 11; days < ip[y]; y--)
+		continue;
+	days -= ip[y];
+	if (mon)
+		*mon = y;
+	if (mday)
+		*mday = days + 1;
+}
+EXPORT_SYMBOL(__offtime);
-- 
1.5.5.3



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v2 2/2] Use common localtime/gmtime in fat_time_unix2fat()
  2009-07-15  7:23   ` [PATCH v2 0/2] " Zhaolei
  2009-07-15  7:24     ` [PATCH v2 1/2] " Zhaolei
@ 2009-07-15  7:25     ` Zhaolei
  1 sibling, 0 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-15  7:25 UTC (permalink / raw)
  To: Andrew Morton; +Cc: mingo, tglx, hirofumi, linux-kernel

It is not necessary to write custom code for convert calendar time
to broken-down time.
localtime()/gmtime() is more generic to do that.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 fs/fat/misc.c |   62 ++++++++++++++++++--------------------------------------
 1 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index a6c2047..d9b6552 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/buffer_head.h>
+#include <linux/time.h>
 #include "fat.h"
 
 /*
@@ -155,10 +156,6 @@ extern struct timezone sys_tz;
 #define SECS_PER_MIN	60
 #define SECS_PER_HOUR	(60 * 60)
 #define SECS_PER_DAY	(SECS_PER_HOUR * 24)
-#define UNIX_SECS_1980	315532800L
-#if BITS_PER_LONG == 64
-#define UNIX_SECS_2108	4354819200L
-#endif
 /* days between 1.1.70 and 1.1.80 (2 leap days) */
 #define DAYS_DELTA	(365 * 10 + 2)
 /* 120 (2100 - 1980) isn't leap year */
@@ -211,58 +208,40 @@ void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
 void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
 		       __le16 *time, __le16 *date, u8 *time_cs)
 {
-	time_t second = ts->tv_sec;
-	time_t day, leap_day, month, year;
-
-	if (!sbi->options.tz_utc)
-		second -= sys_tz.tz_minuteswest * SECS_PER_MIN;
+	unsigned int year, mon, mday, hour, min, sec;
+	if (sbi->options.tz_utc) {
+		gmtime(ts->tv_sec,
+		       &year, &mon, &mday, &hour, &min, &sec, NULL, NULL);
+	} else {
+		localtime(ts->tv_sec,
+		       &year, &mon, &mday, &hour, &min, &sec, NULL, NULL);
+	}
 
-	/* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
-	if (second < UNIX_SECS_1980) {
+	/*  FAT can only support year between 1980 to 2107 */
+	if (year < 1980 - 1900) {
 		*time = 0;
 		*date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
 		if (time_cs)
 			*time_cs = 0;
 		return;
 	}
-#if BITS_PER_LONG == 64
-	if (second >= UNIX_SECS_2108) {
+	if (year > 2107 - 1900) {
 		*time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
 		*date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
 		if (time_cs)
 			*time_cs = 199;
 		return;
 	}
-#endif
 
-	day = second / SECS_PER_DAY - DAYS_DELTA;
-	year = day / 365;
-	leap_day = (year + 3) / 4;
-	if (year > YEAR_2100)		/* 2100 isn't leap year */
-		leap_day--;
-	if (year * 365 + leap_day > day)
-		year--;
-	leap_day = (year + 3) / 4;
-	if (year > YEAR_2100)		/* 2100 isn't leap year */
-		leap_day--;
-	day -= year * 365 + leap_day;
+	/* from 1900 -> from 1980 */
+	year -= 80;
+	/* 0~11 -> 1~12 */
+	mon++;
+	/* 0~59 -> 0~29(2sec counts) */
+	sec >>= 1;
 
-	if (IS_LEAP_YEAR(year) && day == days_in_year[3]) {
-		month = 2;
-	} else {
-		if (IS_LEAP_YEAR(year) && day > days_in_year[3])
-			day--;
-		for (month = 1; month < 12; month++) {
-			if (days_in_year[month + 1] > day)
-				break;
-		}
-	}
-	day -= days_in_year[month];
-
-	*time = cpu_to_le16(((second / SECS_PER_HOUR) % 24) << 11
-			    | ((second / SECS_PER_MIN) % 60) << 5
-			    | (second % SECS_PER_MIN) >> 1);
-	*date = cpu_to_le16((year << 9) | (month << 5) | (day + 1));
+	*time = cpu_to_le16(hour << 11 | min << 5 | sec);
+	*date = cpu_to_le16(year << 9 | mon << 5 | mday);
 	if (time_cs)
 		*time_cs = (ts->tv_sec & 1) * 100 + ts->tv_nsec / 10000000;
 }
@@ -283,4 +262,3 @@ int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
 	}
 	return err;
 }
-
-- 
1.5.5.3



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-14  8:03 [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Zhaolei
  2009-07-14  8:04 ` [PATCH 2/2] fs/fat: Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
  2009-07-14 22:10 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Andrew Morton
@ 2009-07-18 10:02 ` Ingo Molnar
  2009-07-18 12:10   ` H. Peter Anvin
  2009-07-18 12:41   ` Ulrich Drepper
  2009-07-18 12:26 ` Andi Kleen
  3 siblings, 2 replies; 33+ messages in thread
From: Ingo Molnar @ 2009-07-18 10:02 UTC (permalink / raw)
  To: Zhaolei, Ulrich Drepper, H. Peter Anvin
  Cc: Thomas Gleixner, Andrew Morton, OGAWA Hirofumi, linux-kernel


* Zhaolei <zhaolei@cn.fujitsu.com> wrote:

> --- /dev/null
> +++ b/kernel/time/timeconv.c
> @@ -0,0 +1,180 @@
> +/*
> + * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation,  
> Inc.
> + * This file is part of the GNU C Library.
> + * Contributed by Paul Eggert (eggert@twinsun.com).
> + *
> + * The GNU C Library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Library General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * The GNU C Library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Library General Public License for more details.
> + *
> + * You should have received a copy of the GNU Library General Public
> + * License along with the GNU C Library; see the file COPYING.LIB.  If not,
> + * write to the Free Software Foundation, Inc., 59 Temple Place - Suite  
> 330,
> + * Boston, MA 02111-1307, USA.
> + */
> +
> +/*
> + * Converts the calendar time to broken-down time representation
> + * Based on code from glibc-2.6
> + *
> + * 2009-7-14:
> + *   Moved from glibc-2.6 to kernel by Zhaolei<zhaolei@cn.fujitsu.com>
> + */
> +
> +#include <linux/time.h>
> +#include <linux/module.h>
> +
> +/*
> + * Nonzero if YEAR is a leap year (every 4 years,
> + * except every 100th isn't, and every 400th is).
> + */
> +static inline int __isleap(unsigned int year)
> +{
> +    return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
> +}
> +
> +/* How many days come before each month (0-12). */
> +static const unsigned short __mon_yday[2][13] =
> +  {
> +    /* Normal years. */
> +    {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
> +    /* Leap years. */
> +    {0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
> +  };
> +
> +#define SECS_PER_HOUR    (60 * 60)
> +#define SECS_PER_DAY    (SECS_PER_HOUR * 24)
> +
> +static void __offtime(__kernel_time_t totalsecs, int offset,
> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
> +         unsigned int *wday, unsigned int *yday)
> +{
> +    long days, rem, y;
> +    const unsigned short *ip;
> +
> +    days = totalsecs / SECS_PER_DAY;
> +    rem = totalsecs % SECS_PER_DAY;
> +    rem += offset;
> +    while (rem < 0) {
> +        rem += SECS_PER_DAY;
> +        --days;
> +    }
> +    while (rem >= SECS_PER_DAY) {
> +        rem -= SECS_PER_DAY;
> +        ++days;
> +    }
> +
> +    if (hour)
> +        *hour = rem / SECS_PER_HOUR;
> +    rem %= SECS_PER_HOUR;
> +    if (min)
> +        *min = rem / 60;
> +    if (sec)
> +        *sec = rem % 60;
> +
> +    if (wday) {
> +        /* January 1, 1970 was a Thursday. */
> +        *wday = (4 + days) % 7;
> +        if (*wday < 0)
> +            *wday += 7;
> +    }
> +
> +    y = 1970;
> +
> +#define DIV(a, b) ((a) / (b) - ((a) % (b) < 0))
> +#define LEAPS_THRU_END_OF(y) (DIV(y, 4) - DIV(y, 100) + DIV(y, 400))
> +
> +    while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
> +        /* Guess a corrected year, assuming 365 days per year. */
> +        long yg = y + days / 365 - (days % 365 < 0);
> +
> +        /* Adjust DAYS and Y to match the guessed year. */
> +        days -= ((yg - y) * 365
> +             + LEAPS_THRU_END_OF(yg - 1)
> +             - LEAPS_THRU_END_OF(y - 1));
> +        y = yg;
> +    }
> +    if (year) {
> +        *year = y - 1900;
> +        if (*year != y - 1900) {
> +            /* The year cannot be represented due to overflow. */
> +            *year = -1;
> +        }
> +    }
> +
> +    if (yday)
> +        *yday = days;
> +
> +    ip = __mon_yday[__isleap(y)];
> +    for (y = 11; days < ip[y]; y--)
> +        continue;
> +    days -= ip[y];
> +    if (mon)
> +        *mon = y;
> +    if (mday)
> +        *mday = days + 1;
> +}
> +
> +/**
> + * gmtime - converts the calendar time to UTC broken-down time
> + *
> + * @totalsecs    The number of seconds elapsed since 00:00:00 on  
> January 1, 1970,
> + *        Coordinated Universal Time (UTC).
> + * @year    Store the number of years since 1900.
> + * @mon        Store the number of months since January, in the range 0  
> to 11.
> + * @mday    Store the day of the month, in the range 1 to 31.
> + * @hour    Store the number of hours past midnight, in the range 0 to 23.
> + * @min        Store the number of minutes after the hour,
> + *        in the range 0 to 59.
> + * @sec        Store the number of seconds after the minute, normally  
> in the
> + *        range 0 to 59, but can be up to 60 to allow for leap seconds.
> + * @wday    Store the number of days since Sunday, in the range 0 to 6.
> + * @yday    Store the number of days since January 1, in the range 0 to  
> 365.
> + *
> + * Similar to the gmtime() in glibc, broken-down time is expressed in
> + * Coordinated Universal Time (UTC).
> + */
> +void gmtime(__kernel_time_t totalsecs,
> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
> +         unsigned int *wday, unsigned int *yday)
> +{
> +    __offtime(totalsecs, 0, year, mon, mday, hour, min, sec, wday, yday);
> +}
> +EXPORT_SYMBOL(gmtime);
> +
> +/**
> + * localtime - converts the calendar time to local broken-down time
> + *
> + * @totalsecs    The number of seconds elapsed since 00:00:00 on  
> January 1, 1970,
> + *        Coordinated Universal Time (UTC).
> + * @year    Store the number of years since 1900.
> + * @mon        Store the number of months since January, in the range 0  
> to 11.
> + * @mday    Store the day of the month, in the range 1 to 31.
> + * @hour    Store the number of hours past midnight, in the range 0 to 23.
> + * @min        Store the number of minutes after the hour,
> + *        in the range 0 to 59.
> + * @sec        Store the number of seconds after the minute, normally  
> in the
> + *        range 0 to 59, but can be up to 60 to allow for leap seconds.
> + * @wday    Store the number of days since Sunday, in the range 0 to 6.
> + * @yday    Store the number of days since January 1, in the range 0 to  
> 365.
> + *
> + * Similar to the localtime() in glibc, broken-down time is expressed
> + * relative to sys_tz.
> + */
> +void localtime(__kernel_time_t totalsecs,
> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
> +         unsigned int *wday, unsigned int *yday)
> +{
> +    __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, year, mon, mday,  
> hour,
> +        min, sec, wday, yday);
> +}
> +EXPORT_SYMBOL(localtime);

Makes sense - but this code should be converted to the kernel coding 
style. See other code in kernel/time/*.c for examples.

I'm also wondering whether including LGPL glibc code in the kernel 
like this is license-appropriate - there seems to be few precedents 
of it. It _should_ be OK, but i've Cc:-ed involved folks just to 
make sure everyone agrees ...

Since you'll modify it anyway to match up the style, the license 
itself could be restricted to the kernel's GPLv2 subset, that's OK 
as per LGPL.

	Ingo

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-14 22:10 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Andrew Morton
  2009-07-15  0:59   ` Zhaolei
  2009-07-15  7:23   ` [PATCH v2 0/2] " Zhaolei
@ 2009-07-18 11:50   ` Pavel Machek
  2009-07-20  2:56     ` [PATCH 1/2] Add function to convert between calendar time andbroken-down " Zhaolei
  2 siblings, 1 reply; 33+ messages in thread
From: Pavel Machek @ 2009-07-18 11:50 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Zhaolei, mingo, tglx, hirofumi, linux-kernel


> > +extern void gmtime(__kernel_time_t totalsecs,
> > +        unsigned int *year, unsigned int *mon, unsigned int *mday,
> > +        unsigned int *hour, unsigned int *min, unsigned int *sec,
> > +        unsigned int *wday, unsigned int *yday);
> > +extern void localtime(__kernel_time_t totalsecs,
> > +        unsigned int *year, unsigned int *mon, unsigned int *mday,
> > +        unsigned int *hour, unsigned int *min, unsigned int *sec,
> > +        unsigned int *wday, unsigned int *yday);


Should year/mon/.../yday be passed up as a structure?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-18 10:02 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Ingo Molnar
@ 2009-07-18 12:10   ` H. Peter Anvin
  2009-07-18 12:41   ` Ulrich Drepper
  1 sibling, 0 replies; 33+ messages in thread
From: H. Peter Anvin @ 2009-07-18 12:10 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Zhaolei, Ulrich Drepper, Thomas Gleixner, Andrew Morton,
	OGAWA Hirofumi, linux-kernel

Ingo Molnar wrote:
> 
> I'm also wondering whether including LGPL glibc code in the kernel 
> like this is license-appropriate - there seems to be few precedents 
> of it. It _should_ be OK, but i've Cc:-ed involved folks just to 
> make sure everyone agrees ...
> 

Doesn't seem any more odd to me than BSD/GPL dual-licensing, which we 
already have tons of.

	-hpa

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-14  8:03 [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Zhaolei
                   ` (2 preceding siblings ...)
  2009-07-18 10:02 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Ingo Molnar
@ 2009-07-18 12:26 ` Andi Kleen
  2009-07-20  2:41   ` Zhaolei
  3 siblings, 1 reply; 33+ messages in thread
From: Andi Kleen @ 2009-07-18 12:26 UTC (permalink / raw)
  To: Zhaolei
  Cc: Ingo Molnar, Thomas Gleixner, Andrew Morton, OGAWA Hirofumi,
	linux-kernel

Zhaolei <zhaolei@cn.fujitsu.com> writes:
> +
> +static void __offtime(__kernel_time_t totalsecs, int offset,
> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
> +         unsigned int *wday, unsigned int *yday)
> +{
> +    long days, rem, y;
> +    const unsigned short *ip;
> +
> +    days = totalsecs / SECS_PER_DAY;

Does this handle leap seconds? Doesn't seem to.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-18 10:02 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Ingo Molnar
  2009-07-18 12:10   ` H. Peter Anvin
@ 2009-07-18 12:41   ` Ulrich Drepper
  1 sibling, 0 replies; 33+ messages in thread
From: Ulrich Drepper @ 2009-07-18 12:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Zhaolei, H. Peter Anvin, Thomas Gleixner, Andrew Morton,
	OGAWA Hirofumi, linux-kernel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ingo Molnar wrote:
> I'm also wondering whether including LGPL glibc code in the kernel 
> like this is license-appropriate

You can declare the modified code GPLed, no problem.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkphwuUACgkQ2ijCOnn/RHTUFACcD0BZA8AakHERAjnkFOAbYDE0
A7YAnRUrer6z+mRmwXgBBT3/jMUGDBV9
=t0FC
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-18 12:26 ` Andi Kleen
@ 2009-07-20  2:41   ` Zhaolei
  0 siblings, 0 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-20  2:41 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Ingo Molnar, Thomas Gleixner, Andrew Morton, OGAWA Hirofumi,
	linux-kernel

* From: "Andi Kleen" <andi@firstfloor.org>
>> +
>> +static void __offtime(__kernel_time_t totalsecs, int offset,
>> +         unsigned int *year, unsigned int *mon, unsigned int *mday,
>> +         unsigned int *hour, unsigned int *min, unsigned int *sec,
>> +         unsigned int *wday, unsigned int *yday)
>> +{
>> +    long days, rem, y;
>> +    const unsigned short *ip;
>> +
>> +    days = totalsecs / SECS_PER_DAY;
> 
> Does this handle leap seconds? Doesn't seem to.

Hello, Andi

Thank for your attention.

It seems that we don't need to deal with leap seconds in conversion because
unix time is not counting leap seconds..

There is some detail in http://en.wikipedia.org/wiki/Unix_time

Thanks
Zhaolei
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time andbroken-down time for universal use
  2009-07-18 11:50   ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Pavel Machek
@ 2009-07-20  2:56     ` Zhaolei
  2009-07-20  3:20       ` Andrew Morton
  0 siblings, 1 reply; 33+ messages in thread
From: Zhaolei @ 2009-07-20  2:56 UTC (permalink / raw)
  To: Pavel Machek, Andrew Morton; +Cc: mingo, tglx, hirofumi, linux-kernel

* From: "Pavel Machek" <pavel@ucw.cz>
>> > +extern void gmtime(__kernel_time_t totalsecs,
>> > +        unsigned int *year, unsigned int *mon, unsigned int *mday,
>> > +        unsigned int *hour, unsigned int *min, unsigned int *sec,
>> > +        unsigned int *wday, unsigned int *yday);
>> > +extern void localtime(__kernel_time_t totalsecs,
>> > +        unsigned int *year, unsigned int *mon, unsigned int *mday,
>> > +        unsigned int *hour, unsigned int *min, unsigned int *sec,
>> > +        unsigned int *wday, unsigned int *yday);
> 
> 
> Should year/mon/.../yday be passed up as a structure?

Hello, Pavel

Thanks for your attention.

Actually, I considered to introduce a struct as your think, but finally I
choose to use arguments list instead of a struct, because:
1: User can easy to call this function without define a struct
2: Get rid of adding a additional struct into kernel

In fact, I think both(use a struct or not) should be ok.

Thanks
Zhaolei
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] Add function to convert between calendar time andbroken-down time for universal use
  2009-07-20  2:56     ` [PATCH 1/2] Add function to convert between calendar time andbroken-down " Zhaolei
@ 2009-07-20  3:20       ` Andrew Morton
  2009-07-20  9:55         ` [PATCH v3 0/2] Add function to convert between calendar time and broken-down " Zhaolei
  0 siblings, 1 reply; 33+ messages in thread
From: Andrew Morton @ 2009-07-20  3:20 UTC (permalink / raw)
  To: Zhaolei; +Cc: Pavel Machek, mingo, tglx, hirofumi, linux-kernel

On Mon, 20 Jul 2009 10:56:55 +0800 "Zhaolei" <zhaolei@cn.fujitsu.com> wrote:

> * From: "Pavel Machek" <pavel@ucw.cz>
> >> > +extern void gmtime(__kernel_time_t totalsecs,
> >> > +        unsigned int *year, unsigned int *mon, unsigned int *mday,
> >> > +        unsigned int *hour, unsigned int *min, unsigned int *sec,
> >> > +        unsigned int *wday, unsigned int *yday);
> >> > +extern void localtime(__kernel_time_t totalsecs,
> >> > +        unsigned int *year, unsigned int *mon, unsigned int *mday,
> >> > +        unsigned int *hour, unsigned int *min, unsigned int *sec,
> >> > +        unsigned int *wday, unsigned int *yday);
> > 
> > 
> > Should year/mon/.../yday be passed up as a structure?
> 
> Hello, Pavel
> 
> Thanks for your attention.
> 
> Actually, I considered to introduce a struct as your think, but finally I
> choose to use arguments list instead of a struct, because:
> 1: User can easy to call this function without define a struct
> 2: Get rid of adding a additional struct into kernel
> 
> In fact, I think both(use a struct or not) should be ok.

Using a struct will generate better code at caller sites and possibly
at the called site too.  The compiler doesn't need to marshal those
eight pointers on the stack (or wherever the architecture puts them),
possibly less registers will be consumed, etc.  And it'll use less
stack space.

So I do think it would be better from that point of view.

However it will probably require much more code change at the call
sites.  But those changes will be simple, and probably a good thing
regardless.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 0/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-20  3:20       ` Andrew Morton
@ 2009-07-20  9:55         ` Zhaolei
  2009-07-20  9:56           ` [PATCH v3 1/2] " Zhaolei
  2009-07-20  9:57           ` [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
  0 siblings, 2 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-20  9:55 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Pavel Machek, mingo, tglx, hirofumi, linux-kernel

Hello, Andrew

This is Patch v3.
I decide to use a struct instead of argument list to store year/mon/.../yday.

Changelog v2->v3:
Using a struct to save year/mon/.../yday.

Changelog v1->v2:
1: Fix "no tabs" error caused by email client.
2: Remove explicit 'inline' in __isleap()
3: Move DIV() and LEAPS_THRU_END_OF(y) into regular C function along with
   comment
4: Move gmtime() and localtime() into header file.

Thanks
Zhaolei


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-20  9:55         ` [PATCH v3 0/2] Add function to convert between calendar time and broken-down " Zhaolei
@ 2009-07-20  9:56           ` Zhaolei
  2009-07-25  5:42             ` OGAWA Hirofumi
  2009-07-20  9:57           ` [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
  1 sibling, 1 reply; 33+ messages in thread
From: Zhaolei @ 2009-07-20  9:56 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Pavel Machek, mingo, tglx, hirofumi, linux-kernel

There are many similar code in kernel for one object:
convert time between calendar time and broken-down time.

Here is some source I found:
fs/ncpfs/dir.c
fs/smbfs/proc.c
fs/fat/misc.c
fs/udf/udftime.c
fs/cifs/netmisc.c
net/netfilter/xt_time.c
drivers/scsi/ips.c
drivers/input/misc/hp_sdc_rtc.c
drivers/rtc/rtc-lib.c
arch/ia64/hp/sim/boot/fw-emu.c
arch/m68k/mac/misc.c
arch/powerpc/kernel/time.c
arch/parisc/include/asm/rtc.h
...

We can make a common function for this type of convert,
At least we can get following benefit:
1: Make kernel simple and unify
2: Easy to fix bug in converting code
3: Reduce clone of code in future
   For example, I'm trying to make ftrace display walltime,
   this patch will make me easy.

This code is based on code from glibc-2.6

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 include/linux/time.h   |   66 +++++++++++++++++++++++
 kernel/time/Makefile   |    2 +-
 kernel/time/timeconv.c |  138 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 205 insertions(+), 1 deletions(-)
 create mode 100644 kernel/time/timeconv.c

diff --git a/include/linux/time.h b/include/linux/time.h
index ea16c1a..d74bc0c 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -151,6 +151,72 @@ extern void update_xtime_cache(u64 nsec);
 struct tms;
 extern void do_sys_times(struct tms *);
 
+/*
+ * Similar to the struct tm in userspace <time.h>, but it needs to be here so
+ * that the kernel source is self contained.
+ */
+struct tm {
+	/*
+	 * the number of seconds after the minute, normally in the range
+	 * 0 to 59, but can be up to 60 to allow for leap seconds
+	 */
+	int tm_sec;
+	/* the number of minutes after the hour, in the range 0 to 59*/
+	int tm_min;
+	/* the number of hours past midnight, in the range 0 to 23 */
+	int tm_hour;
+	/* the day of the month, in the range 1 to 31 */
+	int tm_mday;
+	/* the number of months since January, in the range 0 to 11 */
+	int tm_mon;
+	/* the number of years since 1900 */
+	int tm_year;
+	/* the number of days since Sunday, in the range 0 to 6 */
+	int tm_wday;
+	/* the number of days since January 1, in the range 0 to 365 */
+	int tm_yday;
+};
+
+extern struct tm *__offtime(__kernel_time_t totalsecs, int offset,
+			    struct tm *result);
+
+/**
+ * gmtime_r - converts the calendar time to UTC broken-down time
+ *
+ * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
+ *		Coordinated Universal Time (UTC).
+ * @result	pointer to struct tm variable to receive broken-down time
+ *
+ * Return a pointer to the broken-down time result on success,
+ * and NULL on when the year does not fit into an integer.
+ *
+ * Similar to the gmtime_r() in glibc, broken-down time is expressed in
+ * Coordinated Universal Time (UTC).
+ */
+static inline struct tm *gmtime_r(__kernel_time_t totalsecs, struct tm *result)
+{
+	return __offtime(totalsecs, 0, result);
+}
+
+/**
+ * localtime_r - converts the calendar time to local broken-down time
+ *
+ * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
+ *		Coordinated Universal Time (UTC).
+ * @result	pointer to struct tm variable to receive broken-down time
+ *
+ * Return a pointer to the broken-down time result on success,
+ * and NULL on when the year does not fit into an integer.
+ *
+ * Similar to the localtime_r() in glibc, broken-down time is expressed
+ * relative to sys_tz.
+ */
+static inline struct tm *localtime_r(__kernel_time_t totalsecs,
+				     struct tm *result)
+{
+	return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
+}
+
 /**
  * timespec_to_ns - Convert timespec to nanoseconds
  * @ts:		pointer to the timespec variable to be converted
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 0b0a636..ee26662 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,4 +1,4 @@
-obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o
+obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o timeconv.o
 
 obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)		+= clockevents.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS)		+= tick-common.o
diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
new file mode 100644
index 0000000..5a76581
--- /dev/null
+++ b/kernel/time/timeconv.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, Inc.
+ * This file is part of the GNU C Library.
+ * Contributed by Paul Eggert (eggert@twinsun.com).
+ *
+ * The GNU C Library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * The GNU C Library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with the GNU C Library; see the file COPYING.LIB.  If not,
+ * write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+/*
+ * Converts the calendar time to broken-down time representation
+ * Based on code from glibc-2.6
+ *
+ * 2009-7-14:
+ *   Moved from glibc-2.6 to kernel by Zhaolei<zhaolei@cn.fujitsu.com>
+ */
+
+#include <linux/time.h>
+#include <linux/module.h>
+
+/*
+ * Nonzero if YEAR is a leap year (every 4 years,
+ * except every 100th isn't, and every 400th is).
+ */
+static int __isleap(unsigned int year)
+{
+	return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
+}
+
+/* do a mathdiv for long type */
+static long math_div(long a, long b)
+{
+	return a / b - (a % b < 0);
+}
+
+/* How many leap years between y1 and y2, y1 must less or equal to y2 */
+static long leaps_between(long y1, long y2)
+{
+	long leaps1 = math_div(y1 - 1, 4) - math_div(y1 - 1, 100)
+		+ math_div(y1 - 1, 400);
+	long leaps2 = math_div(y2 - 1, 4) - math_div(y2 - 1, 100)
+		+ math_div(y2 - 1, 400);
+	return leaps2 - leaps1;
+}
+
+/* How many days come before each month (0-12). */
+static const unsigned short __mon_yday[2][13] = {
+	/* Normal years. */
+	{0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
+	/* Leap years. */
+	{0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
+};
+
+#define SECS_PER_HOUR	(60 * 60)
+#define SECS_PER_DAY	(SECS_PER_HOUR * 24)
+
+/**
+ * __offtime - converts the calendar time to local broken-down time
+ *
+ * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
+ *		Coordinated Universal Time (UTC).
+ * @offset	offset seconds adding to totalsecs.
+ * @result	pointer to struct tm variable to receive broken-down time
+ *
+ * Return a pointer to the broken-down time result on success,
+ * and NULL on when the year does not fit into an integer.
+ *
+ * This function is for internal use, call gmtime_r() and localtime_r() instead.
+ */ 
+struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
+{
+	long days, rem, y;
+	const unsigned short *ip;
+
+	days = totalsecs / SECS_PER_DAY;
+	rem = totalsecs % SECS_PER_DAY;
+	rem += offset;
+	while (rem < 0) {
+		rem += SECS_PER_DAY;
+		--days;
+	}
+	while (rem >= SECS_PER_DAY) {
+		rem -= SECS_PER_DAY;
+		++days;
+	}
+
+	result->tm_hour = rem / SECS_PER_HOUR;
+	rem %= SECS_PER_HOUR;
+	result->tm_min = rem / 60;
+	result->tm_sec = rem % 60;
+
+	/* January 1, 1970 was a Thursday. */
+	result->tm_wday = (4 + days) % 7;
+	if (result->tm_wday < 0)
+		result->tm_wday += 7;
+
+	y = 1970;
+
+	while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
+		/* Guess a corrected year, assuming 365 days per year. */
+		long yg = y + math_div(days, 365);
+
+		/* Adjust DAYS and Y to match the guessed year. */
+		days -= (yg - y) * 365 + leaps_between(y, yg);
+		y = yg;
+	}
+
+	result->tm_year = y - 1900;
+	if (result->tm_year != y - 1900) {
+		/* The year cannot be represented due to overflow. */
+		return NULL;
+	}
+
+	result->tm_yday = days;
+
+	ip = __mon_yday[__isleap(y)];
+	for (y = 11; days < ip[y]; y--)
+		continue;
+	days -= ip[y];
+
+	result->tm_mon = y;
+	result->tm_mday = days + 1;
+
+	return result;
+}
+EXPORT_SYMBOL(__offtime);
-- 
1.5.5.3



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat()
  2009-07-20  9:55         ` [PATCH v3 0/2] Add function to convert between calendar time and broken-down " Zhaolei
  2009-07-20  9:56           ` [PATCH v3 1/2] " Zhaolei
@ 2009-07-20  9:57           ` Zhaolei
  2009-07-20 10:03             ` Pavel Machek
  2009-07-25  5:43             ` OGAWA Hirofumi
  1 sibling, 2 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-20  9:57 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Pavel Machek, mingo, tglx, hirofumi, linux-kernel

It is not necessary to write custom code for convert calendar time
to broken-down time.
localtime()/gmtime() is more generic to do that.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 fs/fat/misc.c |   60 +++++++++++++++++---------------------------------------
 1 files changed, 18 insertions(+), 42 deletions(-)

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index a6c2047..3b9a9d0 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/buffer_head.h>
+#include <linux/time.h>
 #include "fat.h"
 
 /*
@@ -155,10 +156,6 @@ extern struct timezone sys_tz;
 #define SECS_PER_MIN	60
 #define SECS_PER_HOUR	(60 * 60)
 #define SECS_PER_DAY	(SECS_PER_HOUR * 24)
-#define UNIX_SECS_1980	315532800L
-#if BITS_PER_LONG == 64
-#define UNIX_SECS_2108	4354819200L
-#endif
 /* days between 1.1.70 and 1.1.80 (2 leap days) */
 #define DAYS_DELTA	(365 * 10 + 2)
 /* 120 (2100 - 1980) isn't leap year */
@@ -211,58 +208,38 @@ void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
 void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
 		       __le16 *time, __le16 *date, u8 *time_cs)
 {
-	time_t second = ts->tv_sec;
-	time_t day, leap_day, month, year;
-
-	if (!sbi->options.tz_utc)
-		second -= sys_tz.tz_minuteswest * SECS_PER_MIN;
+	struct tm tm;
+	if (sbi->options.tz_utc) {
+		gmtime_r(ts->tv_sec, &tm);
+	} else {
+		localtime_r(ts->tv_sec, &tm);
+	}
 
-	/* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
-	if (second < UNIX_SECS_1980) {
+	/*  FAT can only support year between 1980 to 2107 */
+	if (tm.tm_year < 1980 - 1900) {
 		*time = 0;
 		*date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
 		if (time_cs)
 			*time_cs = 0;
 		return;
 	}
-#if BITS_PER_LONG == 64
-	if (second >= UNIX_SECS_2108) {
+	if (tm.tm_year > 2107 - 1900) {
 		*time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
 		*date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
 		if (time_cs)
 			*time_cs = 199;
 		return;
 	}
-#endif
 
-	day = second / SECS_PER_DAY - DAYS_DELTA;
-	year = day / 365;
-	leap_day = (year + 3) / 4;
-	if (year > YEAR_2100)		/* 2100 isn't leap year */
-		leap_day--;
-	if (year * 365 + leap_day > day)
-		year--;
-	leap_day = (year + 3) / 4;
-	if (year > YEAR_2100)		/* 2100 isn't leap year */
-		leap_day--;
-	day -= year * 365 + leap_day;
+	/* from 1900 -> from 1980 */
+	tm.tm_year -= 80;
+	/* 0~11 -> 1~12 */
+	tm.tm_mon++;
+	/* 0~59 -> 0~29(2sec counts) */
+	tm.tm_sec >>= 1;
 
-	if (IS_LEAP_YEAR(year) && day == days_in_year[3]) {
-		month = 2;
-	} else {
-		if (IS_LEAP_YEAR(year) && day > days_in_year[3])
-			day--;
-		for (month = 1; month < 12; month++) {
-			if (days_in_year[month + 1] > day)
-				break;
-		}
-	}
-	day -= days_in_year[month];
-
-	*time = cpu_to_le16(((second / SECS_PER_HOUR) % 24) << 11
-			    | ((second / SECS_PER_MIN) % 60) << 5
-			    | (second % SECS_PER_MIN) >> 1);
-	*date = cpu_to_le16((year << 9) | (month << 5) | (day + 1));
+	*time = cpu_to_le16(tm.tm_hour << 11 | tm.tm_min << 5 | tm.tm_sec);
+	*date = cpu_to_le16(tm.tm_year << 9 | tm.tm_mon << 5 | tm.tm_mday);
 	if (time_cs)
 		*time_cs = (ts->tv_sec & 1) * 100 + ts->tv_nsec / 10000000;
 }
@@ -283,4 +260,3 @@ int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
 	}
 	return err;
 }
-
-- 
1.5.5.3



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat()
  2009-07-20  9:57           ` [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
@ 2009-07-20 10:03             ` Pavel Machek
  2009-07-25  5:43             ` OGAWA Hirofumi
  1 sibling, 0 replies; 33+ messages in thread
From: Pavel Machek @ 2009-07-20 10:03 UTC (permalink / raw)
  To: Zhaolei; +Cc: Andrew Morton, mingo, tglx, hirofumi, linux-kernel

On Mon 2009-07-20 17:57:22, Zhaolei wrote:
> It is not necessary to write custom code for convert calendar time
> to broken-down time.
> localtime()/gmtime() is more generic to do that.
> 
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>

Ack.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-20  9:56           ` [PATCH v3 1/2] " Zhaolei
@ 2009-07-25  5:42             ` OGAWA Hirofumi
  2009-07-25  8:50               ` OGAWA Hirofumi
                                 ` (3 more replies)
  0 siblings, 4 replies; 33+ messages in thread
From: OGAWA Hirofumi @ 2009-07-25  5:42 UTC (permalink / raw)
  To: Zhaolei; +Cc: Andrew Morton, Pavel Machek, mingo, tglx, linux-kernel

Zhaolei <zhaolei@cn.fujitsu.com> writes:

> +/*
> + * Similar to the struct tm in userspace <time.h>, but it needs to be here so
> + * that the kernel source is self contained.
> + */
> +struct tm {
> +	/*
> +	 * the number of seconds after the minute, normally in the range
> +	 * 0 to 59, but can be up to 60 to allow for leap seconds
> +	 */
> +	int tm_sec;
> +	/* the number of minutes after the hour, in the range 0 to 59*/
> +	int tm_min;
> +	/* the number of hours past midnight, in the range 0 to 23 */
> +	int tm_hour;
> +	/* the day of the month, in the range 1 to 31 */
> +	int tm_mday;
> +	/* the number of months since January, in the range 0 to 11 */
> +	int tm_mon;
> +	/* the number of years since 1900 */
> +	int tm_year;

Why isn't this "long"? "int" can overflow.

> +	/* the number of days since Sunday, in the range 0 to 6 */
> +	int tm_wday;
> +	/* the number of days since January 1, in the range 0 to 365 */
> +	int tm_yday;
> +};

Those are needed? 

> +
> +extern struct tm *__offtime(__kernel_time_t totalsecs, int offset,
> +			    struct tm *result);

Why isn't time_t simply, not __kernel_time_t?

> +/**
> + * gmtime_r - converts the calendar time to UTC broken-down time
> + *
> + * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
> + *		Coordinated Universal Time (UTC).
> + * @result	pointer to struct tm variable to receive broken-down time
> + *
> + * Return a pointer to the broken-down time result on success,
> + * and NULL on when the year does not fit into an integer.
> + *
> + * Similar to the gmtime_r() in glibc, broken-down time is expressed in
> + * Coordinated Universal Time (UTC).
> + */
> +static inline struct tm *gmtime_r(__kernel_time_t totalsecs, struct tm *result)
> +{
> +	return __offtime(totalsecs, 0, result);
> +}
> +
> +/**
> + * localtime_r - converts the calendar time to local broken-down time
> + *
> + * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
> + *		Coordinated Universal Time (UTC).
> + * @result	pointer to struct tm variable to receive broken-down time
> + *
> + * Return a pointer to the broken-down time result on success,
> + * and NULL on when the year does not fit into an integer.
> + *
> + * Similar to the localtime_r() in glibc, broken-down time is expressed
> + * relative to sys_tz.
> + */
> +static inline struct tm *localtime_r(__kernel_time_t totalsecs,
> +				     struct tm *result)
> +{
> +	return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
> +}

I think those are confusable. The real function of those needs to handle
timezone database. Especially, sys_tz.tz_minuteswest in localtime_r() is
known as wrong.

Are you going to fix it? Otherwise I don't think it would not be good to
use it easily as generic function like this.

> +/*
> + * Nonzero if YEAR is a leap year (every 4 years,
> + * except every 100th isn't, and every 400th is).
> + */
> +static int __isleap(unsigned int year)

long year. This breaks negative time_t.

> +{
> +	return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
> +}

> +/**
> + * __offtime - converts the calendar time to local broken-down time
> + *
> + * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
> + *		Coordinated Universal Time (UTC).
> + * @offset	offset seconds adding to totalsecs.
> + * @result	pointer to struct tm variable to receive broken-down time
> + *
> + * Return a pointer to the broken-down time result on success,
> + * and NULL on when the year does not fit into an integer.
> + *
> + * This function is for internal use, call gmtime_r() and localtime_r() instead.
> + */ 
> +struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
> +{

So, I suggest to consolidate this code only, and don't provide
gmtime_r()/localtime_r(), and use more good function name for
__offtime() (I'm not sure, however, personally I feel __offtime is not
obvious what's doing).

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat()
  2009-07-20  9:57           ` [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
  2009-07-20 10:03             ` Pavel Machek
@ 2009-07-25  5:43             ` OGAWA Hirofumi
  2009-07-27  3:21               ` Zhaolei
  1 sibling, 1 reply; 33+ messages in thread
From: OGAWA Hirofumi @ 2009-07-25  5:43 UTC (permalink / raw)
  To: Zhaolei; +Cc: Andrew Morton, Pavel Machek, mingo, tglx, linux-kernel

Zhaolei <zhaolei@cn.fujitsu.com> writes:

> +	if (sbi->options.tz_utc) {
> +		gmtime_r(ts->tv_sec, &tm);
> +	} else {
> +		localtime_r(ts->tv_sec, &tm);
> +	}

Missing error check.

> -	/* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
> -	if (second < UNIX_SECS_1980) {
> +	/*  FAT can only support year between 1980 to 2107 */
> +	if (tm.tm_year < 1980 - 1900) {
>  		*time = 0;
>  		*date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
>  		if (time_cs)
>  			*time_cs = 0;
>  		return;
>  	}
> -#if BITS_PER_LONG == 64
> -	if (second >= UNIX_SECS_2108) {
> +	if (tm.tm_year > 2107 - 1900) {
>  		*time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
>  		*date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
>  		if (time_cs)
>  			*time_cs = 199;
>  		return;
>  	}
> -#endif

I think tm.tm_year is undefine in case of the error.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-25  5:42             ` OGAWA Hirofumi
@ 2009-07-25  8:50               ` OGAWA Hirofumi
  2009-07-25 12:15               ` OGAWA Hirofumi
                                 ` (2 subsequent siblings)
  3 siblings, 0 replies; 33+ messages in thread
From: OGAWA Hirofumi @ 2009-07-25  8:50 UTC (permalink / raw)
  To: Zhaolei; +Cc: Andrew Morton, Pavel Machek, mingo, tglx, linux-kernel

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> Are you going to fix it? Otherwise I don't think it would not be good to
> use it easily as generic function like this.

s/I don't think/I think/
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-25  5:42             ` OGAWA Hirofumi
  2009-07-25  8:50               ` OGAWA Hirofumi
@ 2009-07-25 12:15               ` OGAWA Hirofumi
  2009-07-27  3:15               ` Zhaolei
  2009-07-27 22:44               ` [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use Pavel Machek
  3 siblings, 0 replies; 33+ messages in thread
From: OGAWA Hirofumi @ 2009-07-25 12:15 UTC (permalink / raw)
  To: Zhaolei; +Cc: Andrew Morton, Pavel Machek, mingo, tglx, linux-kernel

[resend: sorry if you got this twice]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> Are you going to fix it? Otherwise I don't think it would not be good to
> use it easily as generic function like this.

s/I don't think/I think/
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-25  5:42             ` OGAWA Hirofumi
  2009-07-25  8:50               ` OGAWA Hirofumi
  2009-07-25 12:15               ` OGAWA Hirofumi
@ 2009-07-27  3:15               ` Zhaolei
  2009-07-27  6:04                 ` OGAWA Hirofumi
  2009-07-27 22:44               ` [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use Pavel Machek
  3 siblings, 1 reply; 33+ messages in thread
From: Zhaolei @ 2009-07-27  3:15 UTC (permalink / raw)
  To: OGAWA Hirofumi, Andrew Morton; +Cc: Pavel Machek, mingo, tglx, linux-kernel

OGAWA Hirofumi wrote:
> Zhaolei <zhaolei@cn.fujitsu.com> writes:
> 
>> +/*
>> + * Similar to the struct tm in userspace <time.h>, but it needs to be here so
>> + * that the kernel source is self contained.
>> + */
>> +struct tm {
>> +	/*
>> +	 * the number of seconds after the minute, normally in the range
>> +	 * 0 to 59, but can be up to 60 to allow for leap seconds
>> +	 */
>> +	int tm_sec;
>> +	/* the number of minutes after the hour, in the range 0 to 59*/
>> +	int tm_min;
>> +	/* the number of hours past midnight, in the range 0 to 23 */
>> +	int tm_hour;
>> +	/* the day of the month, in the range 1 to 31 */
>> +	int tm_mday;
>> +	/* the number of months since January, in the range 0 to 11 */
>> +	int tm_mon;
>> +	/* the number of years since 1900 */
>> +	int tm_year;
> 
> Why isn't this "long"? "int" can overflow.

Hello, OGAWA-san:

Thanks for your review.

I selected int to make it same with user-space struct tm.

In 32-bit machine, long have same length with int, and still can overflow,
It we want to avoid it in all platform, we should use long long,
and make division complex.

Maybe make it same with user-space struct tm is ok.

>> +	/* the number of days since Sunday, in the range 0 to 6 */
>> +	int tm_wday;
>> +	/* the number of days since January 1, in the range 0 to 365 */
>> +	int tm_yday;
>> +};
> 
> Those are needed? 

Those are not needed by FAT-fs's code, but as a library,
I keep them for generic use and keep same with user-space struct tm.

>> +
>> +extern struct tm *__offtime(__kernel_time_t totalsecs, int offset,
>> +			    struct tm *result);
> 
> Why isn't time_t simply, not __kernel_time_t?

Yes, thanks.
It should be time_t.
I will fix it.

>> +/**
>> + * gmtime_r - converts the calendar time to UTC broken-down time
>> + *
>> + * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
>> + *		Coordinated Universal Time (UTC).
>> + * @result	pointer to struct tm variable to receive broken-down time
>> + *
>> + * Return a pointer to the broken-down time result on success,
>> + * and NULL on when the year does not fit into an integer.
>> + *
>> + * Similar to the gmtime_r() in glibc, broken-down time is expressed in
>> + * Coordinated Universal Time (UTC).
>> + */
>> +static inline struct tm *gmtime_r(__kernel_time_t totalsecs, struct tm *result)
>> +{
>> +	return __offtime(totalsecs, 0, result);
>> +}
>> +
>> +/**
>> + * localtime_r - converts the calendar time to local broken-down time
>> + *
>> + * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
>> + *		Coordinated Universal Time (UTC).
>> + * @result	pointer to struct tm variable to receive broken-down time
>> + *
>> + * Return a pointer to the broken-down time result on success,
>> + * and NULL on when the year does not fit into an integer.
>> + *
>> + * Similar to the localtime_r() in glibc, broken-down time is expressed
>> + * relative to sys_tz.
>> + */
>> +static inline struct tm *localtime_r(__kernel_time_t totalsecs,
>> +				     struct tm *result)
>> +{
>> +	return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
>> +}
> 
> I think those are confusable. The real function of those needs to handle
> timezone database. Especially, sys_tz.tz_minuteswest in localtime_r() is
> known as wrong.
> 
> Are you going to fix it? Otherwise I don't think it would not be good to
> use it easily as generic function like this.

Actually, it is hard to select.

I don't schedule to introduce complex timezone database into kernel,
but as you said, localtime() without timezone database is not complete.
But localtime_r is easy to use and understood, it is similar with
userspace same-name function...

>> +/*
>> + * Nonzero if YEAR is a leap year (every 4 years,
>> + * except every 100th isn't, and every 400th is).
>> + */
>> +static int __isleap(unsigned int year)
> 
> long year. This breaks negative time_t.
> 
>> +{
>> +	return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
>> +}
> 
>> +/**
>> + * __offtime - converts the calendar time to local broken-down time
>> + *
>> + * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
>> + *		Coordinated Universal Time (UTC).
>> + * @offset	offset seconds adding to totalsecs.
>> + * @result	pointer to struct tm variable to receive broken-down time
>> + *
>> + * Return a pointer to the broken-down time result on success,
>> + * and NULL on when the year does not fit into an integer.
>> + *
>> + * This function is for internal use, call gmtime_r() and localtime_r() instead.
>> + */ 
>> +struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
>> +{
> 
> So, I suggest to consolidate this code only, and don't provide
> gmtime_r()/localtime_r(), and use more good function name for
> __offtime() (I'm not sure, however, personally I feel __offtime is not
> obvious what's doing).

What about just use gmtime_r(rename __offtime->gmtime_r)?

In fact I think both way(hode original localtime/gmtime or delete them) have
merit and demerit.
Hode them will make it easy to use, delete them will make function more exact.

Hi, Andrew
What is your opinion?

Thanks
Zhaolei


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat()
  2009-07-25  5:43             ` OGAWA Hirofumi
@ 2009-07-27  3:21               ` Zhaolei
  0 siblings, 0 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-27  3:21 UTC (permalink / raw)
  To: OGAWA Hirofumi; +Cc: Andrew Morton, Pavel Machek, mingo, tglx, linux-kernel

OGAWA Hirofumi wrote:
> Zhaolei <zhaolei@cn.fujitsu.com> writes:
> 
>> +	if (sbi->options.tz_utc) {
>> +		gmtime_r(ts->tv_sec, &tm);
>> +	} else {
>> +		localtime_r(ts->tv_sec, &tm);
>> +	}
> 
> Missing error check.
> 
>> -	/* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
>> -	if (second < UNIX_SECS_1980) {
>> +	/*  FAT can only support year between 1980 to 2107 */
>> +	if (tm.tm_year < 1980 - 1900) {
>>  		*time = 0;
>>  		*date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
>>  		if (time_cs)
>>  			*time_cs = 0;
>>  		return;
>>  	}
>> -#if BITS_PER_LONG == 64
>> -	if (second >= UNIX_SECS_2108) {
>> +	if (tm.tm_year > 2107 - 1900) {
>>  		*time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
>>  		*date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
>>  		if (time_cs)
>>  			*time_cs = 199;
>>  		return;
>>  	}
>> -#endif
> 
> I think tm.tm_year is undefine in case of the error.

Hello, Ogawa-san:

Thanks for your review.
I'll fix them

Thanks
Zhaolei



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-27  3:15               ` Zhaolei
@ 2009-07-27  6:04                 ` OGAWA Hirofumi
  2009-07-28  3:05                   ` Zhaolei
  0 siblings, 1 reply; 33+ messages in thread
From: OGAWA Hirofumi @ 2009-07-27  6:04 UTC (permalink / raw)
  To: Zhaolei; +Cc: Andrew Morton, Pavel Machek, mingo, tglx, linux-kernel

Zhaolei <zhaolei@cn.fujitsu.com> writes:

>>> +	/* the number of years since 1900 */
>>> +	int tm_year;
>> 
>> Why isn't this "long"? "int" can overflow.
>
> I selected int to make it same with user-space struct tm.
>
> In 32-bit machine, long have same length with int, and still can overflow,
> It we want to avoid it in all platform, we should use long long,
> and make division complex.
>
> Maybe make it same with user-space struct tm is ok.

time_t is also "long" on 32-bit machine, it does overflow?

>>> +	/* the number of days since Sunday, in the range 0 to 6 */
>>> +	int tm_wday;
>>> +	/* the number of days since January 1, in the range 0 to 365 */
>>> +	int tm_yday;
>>> +};
>> 
>> Those are needed? 
>
> Those are not needed by FAT-fs's code, but as a library,
> I keep them for generic use and keep same with user-space struct tm.

Yes. But, if there is no users, why need? I guess it makes slow thing,
and it is slow than FAT's one (because FAT's one is optimized for
FAT's timestamp range more or less).

>>> +static inline struct tm *localtime_r(__kernel_time_t totalsecs,
>>> +				     struct tm *result)
>>> +{
>>> +	return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
>>> +}
>> 
>> I think those are confusable. The real function of those needs to handle
>> timezone database. Especially, sys_tz.tz_minuteswest in localtime_r() is
>> known as wrong.
>> 
>> Are you going to fix it? Otherwise I don't think it would not be good to
>> use it easily as generic function like this.
>
> Actually, it is hard to select.
>
> I don't schedule to introduce complex timezone database into kernel,
> but as you said, localtime() without timezone database is not complete.
> But localtime_r is easy to use and understood, it is similar with
> userspace same-name function...

Actually it is both (gmtime() is also needed to handle timezone).

I worry someone uses it and display/export to userland. After that, the
user will report the bug of it.

>>> +/*
>>> + * Nonzero if YEAR is a leap year (every 4 years,
>>> + * except every 100th isn't, and every 400th is).
>>> + */
>>> +static int __isleap(unsigned int year)
>> 
>> long year. This breaks negative time_t.

Please "long year".

>>> +struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
>>> +{
>> 
>> So, I suggest to consolidate this code only, and don't provide
>> gmtime_r()/localtime_r(), and use more good function name for
>> __offtime() (I'm not sure, however, personally I feel __offtime is not
>> obvious what's doing).
>
> What about just use gmtime_r(rename __offtime->gmtime_r)?

gmtime() also need to handle timezone actually.

> In fact I think both way(hode original localtime/gmtime or delete them) have
> merit and demerit.
> Hode them will make it easy to use, delete them will make function more exact.

Yes. But, how do you handle bug report?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-25  5:42             ` OGAWA Hirofumi
                                 ` (2 preceding siblings ...)
  2009-07-27  3:15               ` Zhaolei
@ 2009-07-27 22:44               ` Pavel Machek
  2009-07-28  4:52                 ` OGAWA Hirofumi
  3 siblings, 1 reply; 33+ messages in thread
From: Pavel Machek @ 2009-07-27 22:44 UTC (permalink / raw)
  To: OGAWA Hirofumi; +Cc: Zhaolei, Andrew Morton, mingo, tglx, linux-kernel

On Sat 2009-07-25 14:42:06, OGAWA Hirofumi wrote:
> Zhaolei <zhaolei@cn.fujitsu.com> writes:
> 
> > +/*
> > + * Similar to the struct tm in userspace <time.h>, but it needs to be here so
> > + * that the kernel source is self contained.
> > + */
> > +struct tm {
> > +	/*
> > +	 * the number of seconds after the minute, normally in the range
> > +	 * 0 to 59, but can be up to 60 to allow for leap seconds
> > +	 */
> > +	int tm_sec;
> > +	/* the number of minutes after the hour, in the range 0 to 59*/
> > +	int tm_min;
> > +	/* the number of hours past midnight, in the range 0 to 23 */
> > +	int tm_hour;
> > +	/* the day of the month, in the range 1 to 31 */
> > +	int tm_mday;
> > +	/* the number of months since January, in the range 0 to 11 */
> > +	int tm_mon;
> > +	/* the number of years since 1900 */
> > +	int tm_year;
> 
> Why isn't this "long"? "int" can overflow.

Overflow? Year?

I'm not sure how far ahead you are looking, but support for year
2000000000 (2e9) seems perfectly okay to me. (Our friendly star here,
sun, is going to die at cca year 5e9, for comparison. )

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-27  6:04                 ` OGAWA Hirofumi
@ 2009-07-28  3:05                   ` Zhaolei
  2009-07-28  5:12                     ` OGAWA Hirofumi
  0 siblings, 1 reply; 33+ messages in thread
From: Zhaolei @ 2009-07-28  3:05 UTC (permalink / raw)
  To: OGAWA Hirofumi, Andrew Morton, mingo; +Cc: Pavel Machek, tglx, linux-kernel

OGAWA Hirofumi wrote:
> Zhaolei <zhaolei@cn.fujitsu.com> writes:
> 
>>>> +	/* the number of years since 1900 */
>>>> +	int tm_year;
>>> Why isn't this "long"? "int" can overflow.
>> I selected int to make it same with user-space struct tm.
>>
>> In 32-bit machine, long have same length with int, and still can overflow,
>> It we want to avoid it in all platform, we should use long long,
>> and make division complex.
>>
>> Maybe make it same with user-space struct tm is ok.
> 
> time_t is also "long" on 32-bit machine, it does overflow?

Hello, Ogawa-san:
CC: Andrew, ingo:

Yes, time_t on 32-bit machine will overflow on year-2038.
So tm.tm_year will NEVER overflow on 32-bit machine.

And in 64-bit machine, int type of tm.tm_year will overflow in
year 2,147,485,548, Is that enough?

I also like your suggestion to use long for year, so gmtime/localtime
will never return false on both 32 and 64 bit machine.

(btw, I don't understand why time_t is long, it have different length(limit)
in different arch, and make disunity)

>>>> +	/* the number of days since Sunday, in the range 0 to 6 */
>>>> +	int tm_wday;
>>>> +	/* the number of days since January 1, in the range 0 to 365 */
>>>> +	int tm_yday;
>>>> +};
>>> Those are needed? 
>> Those are not needed by FAT-fs's code, but as a library,
>> I keep them for generic use and keep same with user-space struct tm.
> 
> Yes. But, if there is no users, why need?
> I guess it makes slow thing, and it is slow than FAT's one
> (because FAT's one is optimized for FAT's timestamp range more or less).

There is no users of printk("%pf"), why not remove this?
And at least, I found one: drivers/char/efirtc.c
If I continue searching, maybe more.

>>>> +static inline struct tm *localtime_r(__kernel_time_t totalsecs,
>>>> +				     struct tm *result)
>>>> +{
>>>> +	return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
>>>> +}
>>> I think those are confusable. The real function of those needs to handle
>>> timezone database. Especially, sys_tz.tz_minuteswest in localtime_r() is
>>> known as wrong.
>>>
>>> Are you going to fix it? Otherwise I don't think it would not be good to
>>> use it easily as generic function like this.
>> Actually, it is hard to select.
>>
>> I don't schedule to introduce complex timezone database into kernel,
>> but as you said, localtime() without timezone database is not complete.
>> But localtime_r is easy to use and understood, it is similar with
>> userspace same-name function...
> 
> Actually it is both (gmtime() is also needed to handle timezone).
> 
> I worry someone uses it and display/export to userland. After that, the
> user will report the bug of it.

IHMO, user of these functions should understand what these functions is and
use these functions for right way.
If we give user a cdrom, it is user's responsibility not using is as cup stand.

At least, there function can make following source a fairy:
fs/ncpfs/dir.c
fs/smbfs/proc.c
fs/fat/misc.c
fs/udf/udftime.c
fs/cifs/netmisc.c
net/netfilter/xt_time.c
drivers/scsi/ips.c
...

It is different with user-land just because it lakes of large-complex locale
database.

>>>> +/*
>>>> + * Nonzero if YEAR is a leap year (every 4 years,
>>>> + * except every 100th isn't, and every 400th is).
>>>> + */
>>>> +static int __isleap(unsigned int year)
>>> long year. This breaks negative time_t.
> 
> Please "long year".

Ok, or int year(after we get conclusion of long/int).

>>>> +struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
>>>> +{
>>> So, I suggest to consolidate this code only, and don't provide
>>> gmtime_r()/localtime_r(), and use more good function name for
>>> __offtime() (I'm not sure, however, personally I feel __offtime is not
>>> obvious what's doing).
>> What about just use gmtime_r(rename __offtime->gmtime_r)?
> 
> gmtime() also need to handle timezone actually.

So, maybe another function name as unmktime?

>> In fact I think both way(hode original localtime/gmtime or delete them) have
>> merit and demerit.
>> Hode them will make it easy to use, delete them will make function more exact.
> 
> Yes. But, how do you handle bug report?

I will thanks you for attention and reporting first, then discuss on LKML
about detail of this problem, and get a conclusion and fix.

Now we are in step2: discussing.
We need to get a best way to fix this to make users happy.

Thanks
Zhaolei


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-27 22:44               ` [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use Pavel Machek
@ 2009-07-28  4:52                 ` OGAWA Hirofumi
  0 siblings, 0 replies; 33+ messages in thread
From: OGAWA Hirofumi @ 2009-07-28  4:52 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Zhaolei, Andrew Morton, mingo, tglx, linux-kernel

Pavel Machek <pavel@ucw.cz> writes:

>> Why isn't this "long"? "int" can overflow.
>
> Overflow? Year?
>
> I'm not sure how far ahead you are looking, but support for year
> 2000000000 (2e9) seems perfectly okay to me. (Our friendly star here,
> sun, is going to die at cca year 5e9, for comparison. )

Yes. If source is guaranteed as only xtime, but I'd like to think all
case. Especially, because this is library.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-28  3:05                   ` Zhaolei
@ 2009-07-28  5:12                     ` OGAWA Hirofumi
  2009-07-30  5:39                       ` [PATCH v4 0/2] " Zhaolei
  0 siblings, 1 reply; 33+ messages in thread
From: OGAWA Hirofumi @ 2009-07-28  5:12 UTC (permalink / raw)
  To: Zhaolei; +Cc: Andrew Morton, mingo, Pavel Machek, tglx, linux-kernel

Zhaolei <zhaolei@cn.fujitsu.com> writes:

> There is no users of printk("%pf"), why not remove this?

IMO, because it doesn't make slow thing.

> And at least, I found one: drivers/char/efirtc.c
> If I continue searching, maybe more.

If there is a user, I think it's ok.

> IHMO, user of these functions should understand what these functions is and
> use these functions for right way.
> If we give user a cdrom, it is user's responsibility not using is as cup stand.
>
> At least, there function can make following source a fairy:
> fs/ncpfs/dir.c
> fs/smbfs/proc.c
> fs/fat/misc.c
> fs/udf/udftime.c
> fs/cifs/netmisc.c
> net/netfilter/xt_time.c
> drivers/scsi/ips.c
> ...
>
> It is different with user-land just because it lakes of large-complex locale
> database.

Yes, and I'm understanding FAT has the bug, and there is actual bug
report (bug FAT requires this as on-disk format). However, my point is
why we increase the known bugs.

>> gmtime() also need to handle timezone actually.
>
> So, maybe another function name as unmktime?

Maybe. Um..., or time_to_tm()? I'm not sure.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 0/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-28  5:12                     ` OGAWA Hirofumi
@ 2009-07-30  5:39                       ` Zhaolei
  2009-07-30  5:40                         ` [PATCH v4 1/2] " Zhaolei
  2009-07-30  5:41                         ` [PATCH v4 2/2] Use common time_to_tm in fat_time_unix2fat() Zhaolei
  0 siblings, 2 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-30  5:39 UTC (permalink / raw)
  To: Andrew Morton, mingo; +Cc: OGAWA Hirofumi, Pavel Machek, tglx, linux-kernel

Hello, Andrew

This is Patch v4.
I updated this patch bases on Ogawa-san's suggestion.

Changelog v3->v4:
1: Use long instead of int for tm.tm_year to avoid overflow
2: Remove localtime() and gmtime() because they can't support timezone
   database as user-more. And rename __offtime() to time_to_tm()
3: Use time_t instead of __kernel_time_t as function's argument type
4: __isleap(unsigned int year)->__isleap(long year) to avoid overflow

Changelog v2->v3:
Using a struct to save year/mon/.../yday.

Changelog v1->v2:
1: Fix "no tabs" error caused by email client.
2: Remove explicit 'inline' in __isleap()
3: Move DIV() and LEAPS_THRU_END_OF(y) into regular C function along with
   comment
4: Move gmtime() and localtime() into header file.

Thanks
Zhaolei


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 1/2] Add function to convert between calendar time and broken-down time for universal use
  2009-07-30  5:39                       ` [PATCH v4 0/2] " Zhaolei
@ 2009-07-30  5:40                         ` Zhaolei
  2009-07-30  5:41                         ` [PATCH v4 2/2] Use common time_to_tm in fat_time_unix2fat() Zhaolei
  1 sibling, 0 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-30  5:40 UTC (permalink / raw)
  To: Andrew Morton, mingo; +Cc: OGAWA Hirofumi, Pavel Machek, tglx, linux-kernel

There are many similar code in kernel for one object:
convert time between calendar time and broken-down time.

Here is some source I found:
fs/ncpfs/dir.c
fs/smbfs/proc.c
fs/fat/misc.c
fs/udf/udftime.c
fs/cifs/netmisc.c
net/netfilter/xt_time.c
drivers/scsi/ips.c
drivers/input/misc/hp_sdc_rtc.c
drivers/rtc/rtc-lib.c
arch/ia64/hp/sim/boot/fw-emu.c
arch/m68k/mac/misc.c
arch/powerpc/kernel/time.c
arch/parisc/include/asm/rtc.h
...

We can make a common function for this type of convert,
At least we can get following benefit:
1: Make kernel simple and unify
2: Easy to fix bug in converting code
3: Reduce clone of code in future
   For example, I'm trying to make ftrace display walltime,
   this patch will make me easy.

This code is based on code from glibc-2.6

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 include/linux/time.h   |   28 +++++++++++
 kernel/time/Makefile   |    2 +-
 kernel/time/timeconv.c |  127 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+), 1 deletions(-)
 create mode 100644 kernel/time/timeconv.c

diff --git a/include/linux/time.h b/include/linux/time.h
index ea16c1a..4bc7a91 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -151,6 +151,34 @@ extern void update_xtime_cache(u64 nsec);
 struct tms;
 extern void do_sys_times(struct tms *);
 
+/*
+ * Similar to the struct tm in userspace <time.h>, but it needs to be here so
+ * that the kernel source is self contained.
+ */
+struct tm {
+	/*
+	 * the number of seconds after the minute, normally in the range
+	 * 0 to 59, but can be up to 60 to allow for leap seconds
+	 */
+	int tm_sec;
+	/* the number of minutes after the hour, in the range 0 to 59*/
+	int tm_min;
+	/* the number of hours past midnight, in the range 0 to 23 */
+	int tm_hour;
+	/* the day of the month, in the range 1 to 31 */
+	int tm_mday;
+	/* the number of months since January, in the range 0 to 11 */
+	int tm_mon;
+	/* the number of years since 1900 */
+	long tm_year;
+	/* the number of days since Sunday, in the range 0 to 6 */
+	int tm_wday;
+	/* the number of days since January 1, in the range 0 to 365 */
+	int tm_yday;
+};
+
+void time_to_tm(time_t totalsecs, int offset, struct tm *result);
+
 /**
  * timespec_to_ns - Convert timespec to nanoseconds
  * @ts:		pointer to the timespec variable to be converted
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 0b0a636..ee26662 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,4 +1,4 @@
-obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o
+obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o timeconv.o
 
 obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)		+= clockevents.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS)		+= tick-common.o
diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
new file mode 100644
index 0000000..86628e7
--- /dev/null
+++ b/kernel/time/timeconv.c
@@ -0,0 +1,127 @@
+/*
+ * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, Inc.
+ * This file is part of the GNU C Library.
+ * Contributed by Paul Eggert (eggert@twinsun.com).
+ *
+ * The GNU C Library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * The GNU C Library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with the GNU C Library; see the file COPYING.LIB.  If not,
+ * write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+/*
+ * Converts the calendar time to broken-down time representation
+ * Based on code from glibc-2.6
+ *
+ * 2009-7-14:
+ *   Moved from glibc-2.6 to kernel by Zhaolei<zhaolei@cn.fujitsu.com>
+ */
+
+#include <linux/time.h>
+#include <linux/module.h>
+
+/*
+ * Nonzero if YEAR is a leap year (every 4 years,
+ * except every 100th isn't, and every 400th is).
+ */
+static int __isleap(long year)
+{
+	return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
+}
+
+/* do a mathdiv for long type */
+static long math_div(long a, long b)
+{
+	return a / b - (a % b < 0);
+}
+
+/* How many leap years between y1 and y2, y1 must less or equal to y2 */
+static long leaps_between(long y1, long y2)
+{
+	long leaps1 = math_div(y1 - 1, 4) - math_div(y1 - 1, 100)
+		+ math_div(y1 - 1, 400);
+	long leaps2 = math_div(y2 - 1, 4) - math_div(y2 - 1, 100)
+		+ math_div(y2 - 1, 400);
+	return leaps2 - leaps1;
+}
+
+/* How many days come before each month (0-12). */
+static const unsigned short __mon_yday[2][13] = {
+	/* Normal years. */
+	{0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
+	/* Leap years. */
+	{0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
+};
+
+#define SECS_PER_HOUR	(60 * 60)
+#define SECS_PER_DAY	(SECS_PER_HOUR * 24)
+
+/**
+ * time_to_tm - converts the calendar time to local broken-down time
+ *
+ * @totalsecs	the number of seconds elapsed since 00:00:00 on January 1, 1970,
+ *		Coordinated Universal Time (UTC).
+ * @offset	offset seconds adding to totalsecs.
+ * @result	pointer to struct tm variable to receive broken-down time
+ */
+void time_to_tm(time_t totalsecs, int offset, struct tm *result)
+{
+	long days, rem, y;
+	const unsigned short *ip;
+
+	days = totalsecs / SECS_PER_DAY;
+	rem = totalsecs % SECS_PER_DAY;
+	rem += offset;
+	while (rem < 0) {
+		rem += SECS_PER_DAY;
+		--days;
+	}
+	while (rem >= SECS_PER_DAY) {
+		rem -= SECS_PER_DAY;
+		++days;
+	}
+
+	result->tm_hour = rem / SECS_PER_HOUR;
+	rem %= SECS_PER_HOUR;
+	result->tm_min = rem / 60;
+	result->tm_sec = rem % 60;
+
+	/* January 1, 1970 was a Thursday. */
+	result->tm_wday = (4 + days) % 7;
+	if (result->tm_wday < 0)
+		result->tm_wday += 7;
+
+	y = 1970;
+
+	while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
+		/* Guess a corrected year, assuming 365 days per year. */
+		long yg = y + math_div(days, 365);
+
+		/* Adjust DAYS and Y to match the guessed year. */
+		days -= (yg - y) * 365 + leaps_between(y, yg);
+		y = yg;
+	}
+
+	result->tm_year = y - 1900;
+
+	result->tm_yday = days;
+
+	ip = __mon_yday[__isleap(y)];
+	for (y = 11; days < ip[y]; y--)
+		continue;
+	days -= ip[y];
+
+	result->tm_mon = y;
+	result->tm_mday = days + 1;
+}
+EXPORT_SYMBOL(time_to_tm);
-- 
1.5.5.3



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 2/2] Use common time_to_tm in fat_time_unix2fat()
  2009-07-30  5:39                       ` [PATCH v4 0/2] " Zhaolei
  2009-07-30  5:40                         ` [PATCH v4 1/2] " Zhaolei
@ 2009-07-30  5:41                         ` Zhaolei
  1 sibling, 0 replies; 33+ messages in thread
From: Zhaolei @ 2009-07-30  5:41 UTC (permalink / raw)
  To: Andrew Morton, mingo; +Cc: OGAWA Hirofumi, Pavel Machek, tglx, linux-kernel

It is not necessary to write custom code for convert calendar time
to broken-down time.
time_to_tm() is more generic to do that.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 fs/fat/misc.c |   57 +++++++++++++++------------------------------------------
 1 files changed, 15 insertions(+), 42 deletions(-)

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index a6c2047..03bf98d 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/buffer_head.h>
+#include <linux/time.h>
 #include "fat.h"
 
 /*
@@ -155,10 +156,6 @@ extern struct timezone sys_tz;
 #define SECS_PER_MIN	60
 #define SECS_PER_HOUR	(60 * 60)
 #define SECS_PER_DAY	(SECS_PER_HOUR * 24)
-#define UNIX_SECS_1980	315532800L
-#if BITS_PER_LONG == 64
-#define UNIX_SECS_2108	4354819200L
-#endif
 /* days between 1.1.70 and 1.1.80 (2 leap days) */
 #define DAYS_DELTA	(365 * 10 + 2)
 /* 120 (2100 - 1980) isn't leap year */
@@ -211,58 +208,35 @@ void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
 void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
 		       __le16 *time, __le16 *date, u8 *time_cs)
 {
-	time_t second = ts->tv_sec;
-	time_t day, leap_day, month, year;
+	struct tm tm;
+	time_to_tm(ts->tv_sec, sbi->options.tz_utc ? 0 :
+		   -sys_tz.tz_minuteswest * 60, &tm);
 
-	if (!sbi->options.tz_utc)
-		second -= sys_tz.tz_minuteswest * SECS_PER_MIN;
-
-	/* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
-	if (second < UNIX_SECS_1980) {
+	/*  FAT can only support year between 1980 to 2107 */
+	if (tm.tm_year < 1980 - 1900) {
 		*time = 0;
 		*date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
 		if (time_cs)
 			*time_cs = 0;
 		return;
 	}
-#if BITS_PER_LONG == 64
-	if (second >= UNIX_SECS_2108) {
+	if (tm.tm_year > 2107 - 1900) {
 		*time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
 		*date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
 		if (time_cs)
 			*time_cs = 199;
 		return;
 	}
-#endif
 
-	day = second / SECS_PER_DAY - DAYS_DELTA;
-	year = day / 365;
-	leap_day = (year + 3) / 4;
-	if (year > YEAR_2100)		/* 2100 isn't leap year */
-		leap_day--;
-	if (year * 365 + leap_day > day)
-		year--;
-	leap_day = (year + 3) / 4;
-	if (year > YEAR_2100)		/* 2100 isn't leap year */
-		leap_day--;
-	day -= year * 365 + leap_day;
-
-	if (IS_LEAP_YEAR(year) && day == days_in_year[3]) {
-		month = 2;
-	} else {
-		if (IS_LEAP_YEAR(year) && day > days_in_year[3])
-			day--;
-		for (month = 1; month < 12; month++) {
-			if (days_in_year[month + 1] > day)
-				break;
-		}
-	}
-	day -= days_in_year[month];
+	/* from 1900 -> from 1980 */
+	tm.tm_year -= 80;
+	/* 0~11 -> 1~12 */
+	tm.tm_mon++;
+	/* 0~59 -> 0~29(2sec counts) */
+	tm.tm_sec >>= 1;
 
-	*time = cpu_to_le16(((second / SECS_PER_HOUR) % 24) << 11
-			    | ((second / SECS_PER_MIN) % 60) << 5
-			    | (second % SECS_PER_MIN) >> 1);
-	*date = cpu_to_le16((year << 9) | (month << 5) | (day + 1));
+	*time = cpu_to_le16(tm.tm_hour << 11 | tm.tm_min << 5 | tm.tm_sec);
+	*date = cpu_to_le16(tm.tm_year << 9 | tm.tm_mon << 5 | tm.tm_mday);
 	if (time_cs)
 		*time_cs = (ts->tv_sec & 1) * 100 + ts->tv_nsec / 10000000;
 }
@@ -283,4 +257,3 @@ int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
 	}
 	return err;
 }
-
-- 
1.5.5.3



^ permalink raw reply related	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2009-07-30  5:38 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-14  8:03 [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Zhaolei
2009-07-14  8:04 ` [PATCH 2/2] fs/fat: Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
2009-07-14 22:10 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Andrew Morton
2009-07-15  0:59   ` Zhaolei
2009-07-15  7:23   ` [PATCH v2 0/2] " Zhaolei
2009-07-15  7:24     ` [PATCH v2 1/2] " Zhaolei
2009-07-15  7:25     ` [PATCH v2 2/2] Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
2009-07-18 11:50   ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Pavel Machek
2009-07-20  2:56     ` [PATCH 1/2] Add function to convert between calendar time andbroken-down " Zhaolei
2009-07-20  3:20       ` Andrew Morton
2009-07-20  9:55         ` [PATCH v3 0/2] Add function to convert between calendar time and broken-down " Zhaolei
2009-07-20  9:56           ` [PATCH v3 1/2] " Zhaolei
2009-07-25  5:42             ` OGAWA Hirofumi
2009-07-25  8:50               ` OGAWA Hirofumi
2009-07-25 12:15               ` OGAWA Hirofumi
2009-07-27  3:15               ` Zhaolei
2009-07-27  6:04                 ` OGAWA Hirofumi
2009-07-28  3:05                   ` Zhaolei
2009-07-28  5:12                     ` OGAWA Hirofumi
2009-07-30  5:39                       ` [PATCH v4 0/2] " Zhaolei
2009-07-30  5:40                         ` [PATCH v4 1/2] " Zhaolei
2009-07-30  5:41                         ` [PATCH v4 2/2] Use common time_to_tm in fat_time_unix2fat() Zhaolei
2009-07-27 22:44               ` [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use Pavel Machek
2009-07-28  4:52                 ` OGAWA Hirofumi
2009-07-20  9:57           ` [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat() Zhaolei
2009-07-20 10:03             ` Pavel Machek
2009-07-25  5:43             ` OGAWA Hirofumi
2009-07-27  3:21               ` Zhaolei
2009-07-18 10:02 ` [PATCH 1/2] Add function to convert between calendar time and broken-down time for universal use Ingo Molnar
2009-07-18 12:10   ` H. Peter Anvin
2009-07-18 12:41   ` Ulrich Drepper
2009-07-18 12:26 ` Andi Kleen
2009-07-20  2:41   ` Zhaolei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox