Hi Rafael,

Rafael.Vanoni <> wrote:

> I've posted a webrev at http://cr.opensolaris.org/~rafaelv/ptop-3bugs/
> with fixes for
> 
> 11863 i18n and pt_error()
> 2861 C0 residency sits at 100% and all wakeups are 0.0% when interval
> is large
> 11862 powertop(1) reports incorrect c-state data for sun4v systems
> 
> Each bug report has its own patch, but they are different enough that
> you can spot individual changes in the webrev. Please have a look and
> let me know what you think.
> 

Two comments about the webrev

1)  I'm still a bit confused why we need this change. Looking at the dtrace
probe in the kernel, 

DTRACE_PROBE1(idle__state__transition, uint_t, state);

The type of state is uint_t. No matter on 32-bit or 64-bit system,
the size of the type should be 4. The patch proposed 8 on 64-bit
system, I have no idea about SPARC, but this is not correct on IA.
Did I miss something?

-       /* LINTED - alignment */
-       state = *(int32_t *)(data->dtada_data + rec->dtrd_offset);
 
+       switch (g_bit_depth) {
+               case 32:
+                       /* LINTED - alignment */
+                       state = *(uint32_t *)(data->dtada_data +
+                           rec->dtrd_offset);
+                       break;
+               case 64:
+                       /* LINTED - alignment */
+                       state = *(uint64_t *)(data->dtada_data +
+                           rec->dtrd_offset);
+                       break;
+       }
+

2) for what reason we need to do the change like this?

-#define        INTERVAL_MAX            100.0
+#define        INTERVAL_MAX            30.0

-           "data in seconds [1-100s]\n");
+           "data in seconds [1-30s]\n");

Thanks,
-Aubrey

Reply via email to