在 18/4/12 上午10:50, Waldek Kozaczuk 写道:
Including the author of the patch.

Sent from my iPhone

On Apr 10, 2018, at 23:44, Waldek Kozaczuk <jwkozac...@gmail.com <mailto:jwkozac...@gmail.com>> wrote:

Nice catch.

Even though the patch addresses the bug it also makes _event member variable public and therefore weakens encapsulation. I wonder if we can address this bug along these lines:
i agree it, thanks for your review,
then i commit v2 patch, and add reviewer Waldek Kozaczuk <jwkozac...@gmail.com>
ok?

diff --git a/arch/x64/arch-setup.cc <http://arch-setup.cc> b/arch/x64/arch-setup.cc <http://arch-setup.cc>
index 325c26a..e4c1ab0 100644
--- a/arch/x64/arch-setup.cc <http://arch-setup.cc>
+++ b/arch/x64/arch-setup.cc <http://arch-setup.cc>
@@ -134,11 +134,11 @@ void arch_setup_free_memory()
     u64 time;
     time = omb.tsc_init_hi;
     time = (time << 32) | omb.tsc_init;
-    boot_time.arrays[0] = { "", time };
+    boot_time.event(0, "", time );
     time = omb.tsc_disk_done_hi;
     time = (time << 32) | omb.tsc_disk_done;
-    boot_time.arrays[1] = { "disk read (real mode)", time };
+    boot_time.event(1, "disk read (real mode)", time );
     auto c = processor::cpuid(0x80000000);
     if (c.a >= 0x80000008) {
diff --git a/core/chart.cc <http://chart.cc> b/core/chart.cc <http://chart.cc>
index 86153a2..59311a9 100644
--- a/core/chart.cc <http://chart.cc>
+++ b/core/chart.cc <http://chart.cc>
@@ -20,8 +20,18 @@ void boot_time_chart::print_one_time(int index)
 void boot_time_chart::event(const char *str)
 {
-    arrays[_event].str  = str;
-    arrays[_event++].stamp = processor::ticks();
+    event(_event++, str, processor::ticks());
+}
+
+void boot_time_chart::event(int event_idx, const char *str)
+{
+    event(event_idx, str, processor::ticks());
+}
+
+void boot_time_chart::event(int event_idx, const char *str, u64 stamp)
+{
+    arrays[event_idx].str = str;
+    arrays[event_idx].stamp = stamp;
 }
 void boot_time_chart::print_chart()
diff --git a/include/osv/boot.hh b/include/osv/boot.hh
index eb97cc9..391ad8f 100644
--- a/include/osv/boot.hh
+++ b/include/osv/boot.hh
@@ -12,16 +12,17 @@ public:
 class boot_time_chart {
 public:
     void event(const char *str);
+    void event(int event_idx, const char *str);
+    void event(int event_idx, const char *str, u64 stamp);
     void print_chart();
-    time_element arrays[16];
-    friend void arch_setup_free_memory();
 private:
// Can we keep it at 0 and let the initial two users increment it? No, we // cannot. The reason is that the code that *parses* those fields run // relatively late (the code that takes the measure is so early it cannot // call this one directly. Therefore, the measurements would appear in the
     // middle of the list, and we want to preserve order.
-    int _event = 2;
+    int _event = 3;
+    time_element arrays[16];
     void print_one_time(int index);
     double to_msec(u64 time);
diff --git a/loader.cc <http://loader.cc> b/loader.cc <http://loader.cc>
index f6cbd4d..de1d772 100644
--- a/loader.cc <http://loader.cc>
+++ b/loader.cc <http://loader.cc>
@@ -109,7 +109,8 @@ void premain()
     }
     setup_tls(inittab);
-    boot_time.event("TLS initialization");
+    debug("After TLS initialization");
+    boot_time.event(2, "TLS initialization");
for (auto init = inittab.start; init < inittab.start + inittab.count; ++init) {
         (*init)();
     }


Waldek


On Monday, April 9, 2018 at 8:17:11 AM UTC-4, Wang Yu wrote:

    before patch boot with --bootchart, "TLS initialization"
    is not output
            ...
            disk read (real mode): 50.71ms, (+50.71ms)
            .init functions: 89.20ms, (+5.48ms)
            SMP launched: 90.56ms, (+1.36ms)
            VFS initialized: 95.76ms, (+5.20ms)
            ...
    after patch,
            ...
            disk read (real mode): 50.71ms, (+50.71ms)
            TLS initialization: 83.72ms, (+33.01ms)
            .init functions: 89.20ms, (+5.48ms)
            SMP launched: 90.56ms, (+1.36ms)
            VFS initialized: 95.76ms, (+5.20ms)
            ...
    Signed-off-by: Wang Yu <yuw...@linux.alibaba.com>
    ---
     include/osv/boot.hh | 4 ++--
    loader.cc <http://loader.cc>           | 1 +
     2 files changed, 3 insertions(+), 2 deletions(-)

    diff --git a/include/osv/boot.hh b/include/osv/boot.hh
    index eb97cc9..ec5301c 100644
    --- a/include/osv/boot.hh
    +++ b/include/osv/boot.hh
    @@ -15,13 +15,13 @@ public:
         void print_chart();
         time_element arrays[16];
         friend void arch_setup_free_memory();
    -private:
         // Can we keep it at 0 and let the initial two users
    increment it?  No, we
         // cannot. The reason is that the code that *parses* those
    fields run
         // relatively late (the code that takes the measure is so
    early it cannot
         // call this one directly. Therefore, the measurements would
    appear in the
         // middle of the list, and we want to preserve order.
    -    int _event = 2;
    +    int _event = 3;
    +private:

         void print_one_time(int index);
         double to_msec(u64 time);
    diff --git a/loader.cc <http://loader.cc> b/loader.cc
    <http://loader.cc>
    index f6cbd4d..0d97151 100644
    --- a/loader.cc <http://loader.cc>
    +++ b/loader.cc <http://loader.cc>
    @@ -109,6 +109,7 @@ void premain()
         }

         setup_tls(inittab);
    +    boot_time._event=2;
         boot_time.event("TLS initialization");
         for (auto init = inittab.start; init < inittab.start +
    inittab.count; ++init) {
             (*init)();
-- 1.8.3.1

--
You received this message because you are subscribed to a topic in the Google Groups "OSv Development" group. To unsubscribe from this topic, visit https://groups.google.com/d/topic/osv-dev/XWBt6oDA6_4/unsubscribe. To unsubscribe from this group and all its topics, send an email to osv-dev+unsubscr...@googlegroups.com <mailto:osv-dev+unsubscr...@googlegroups.com>.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to