Re: [Lse-tech] [Patch 6/8] delay accounting usage of taskstats interface

From: Jay Lan
Date: Wed May 03 2006 - 22:15:54 EST


Balbir Singh wrote:
>Changelog
>
>Fixes suggested by Jay Lan
>- check for tidstats before taking the mutex_lock in taskstats_exit_send()
>- add back version information for struct taskstats
>
><clip>
>
> struct taskstats {
>
>- /* Version 1 */
>+ /* Delay accounting fields start
>+ *
>+ * All values, until comment "Delay accounting fields end" are
>+ * available only if delay accounting is enabled, even though the last
>+ * few fields are not delays
>+ *
>+ * xxx_count is the number of delay values recorded
>+ * xxx_delay_total is the corresponding cumulative delay in nanoseconds
>+ *
>+ * xxx_delay_total wraps around to zero on overflow
>+ * xxx_count incremented regardless of overflow
>+ */
>+
>+ /* Delay waiting for cpu, while runnable
>+ * count, delay_total NOT updated atomically
>+ */
>+ __u64 cpu_count;
>+ __u64 cpu_delay_total;
>+
>+ /* Following four fields atomically updated using task->delays->lock */
>+
>+ /* Delay waiting for synchronous block I/O to complete
>+ * does not account for delays in I/O submission
>+ */
>+ __u64 blkio_count;
>+ __u64 blkio_delay_total;
>+
>+ /* Delay waiting for page fault I/O (swap in only) */
>+ __u64 swapin_count;
>+ __u64 swapin_delay_total;
>+
>+ /* cpu "wall-clock" running time
>+ * On some architectures, value will adjust for cpu time stolen
>+ * from the kernel in involuntary waits due to virtualization.
>+ * Value is cumulative, in nanoseconds, without a corresponding count
>+ * and wraps around to zero silently on overflow
>+ */
>+ __u64 cpu_run_real_total;
>+
>+ /* cpu "virtual" running time
>+ * Uses time intervals seen by the kernel i.e. no adjustment
>+ * for kernel's involuntary waits due to virtualization.
>+ * Value is cumulative, in nanoseconds, without a corresponding count
>+ * and wraps around to zero silently on overflow
>+ */
>+ __u64 cpu_run_virtual_total;
>+ /* Delay accounting fields end */
>+ /* version 1 ends here */
>+
>+ /* version of taskstats */
>+ __u64 version;
>

Could you place the common field "version" before any acct subsystem
specific fields?

As a matter of fact, we do not need
'filler_avoids_empty_struct_warnings' in [patch 5/8] taskstats
interface. Replacing that field with "version" would be great!

Thanks,
- jay


>
>- int filler_avoids_empty_struct_warnings;
> };
>
>
>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/