[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 5/5] xen/console: Traditional console timestamps including milliseconds



On 03/06/14 19:41, Andrew Cooper wrote:
On 06/03/2014 23:54, Don Slutz wrote:
On 03/06/14 14:28, Andrew Cooper wrote:
   }
   -struct tm wallclock_time(void)
+struct tm wallclock_time(uint64_t *ns)
   {
Adding:

    if ( ns )
       *ns = 0;

Makes sense here.

       return (struct tm) { 0 };
   }
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 4f4de22..1156ccc 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1646,15 +1646,19 @@ int dom0_pit_access(struct ioreq *ioreq)
       return 0;
   }
   -struct tm wallclock_time(void)
+struct tm wallclock_time(uint64_t *ns)
   {
-    uint64_t seconds;
+    uint64_t seconds, nsec;
         if ( !wc_sec )
And here.

           return (struct tm) { 0 };
         seconds = NOW() + SECONDS(wc_sec) + wc_nsec;
-    do_div(seconds, 1000000000);
+    nsec = do_div(seconds, 1000000000);
+
+    if ( *ns )
This should be just

    if ( ns )
Oops - so it should.

As for the other changes, I am a bit ambivalent.  printk_start_of_line()
is the sole caller of wallclock_time(), which means that tm.tm_day being
0 means *ns will never get looked at.

This is not 100% true.  Last I knew (struct tm) { 0 } is

struct tm {
    int     tm_sec;         /* seconds */ = 0
    int     tm_min;         /* minutes */ <undef>
    int     tm_hour;        /* hours */ <undef>
    int     tm_mday;        /* day of the month */ <undef>
    int     tm_mon;         /* month */ <undef>
    int     tm_year;        /* year */ <undef>
    int     tm_wday;        /* day of the week */ <undef>
    int     tm_yday;        /* day in the year */ <undef>
    int     tm_isdst;       /* daylight saving time */ <undef>
};

But not having looked at the code, and the compilers could have changed what this means (i.e. if not provided they are zero in which case this should be {}. { 0, 0, 0, 0 } or { .tm_mday = 0 } is the way I know of to say "tm_mday" is zero).

So the test for tm.tm_mday == 0 may just be working because the undefined value is zero...

While it is not exactly a hot codepath, unconditionally clearing it
seems silly, especially as it is not exactly the most likely candidate
to get a new caller in the near future.

There is no clear right answer here. So I have no issue with not doing the "extra" work.

   -Don Slutz

~Andrew



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.