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

[Jay Lan <jlan@engr.sgi.com>]

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;
> };
> 
> 
>