Message-ID: <44596362.3080207@engr.sgi.com>
Date: Wed, 03 May 2006 19:13:54 -0700
From: Jay Lan <jlan@engr.sgi.com>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20050921
MIME-Version: 1.0
To: balbir@in.ibm.com
CC: linux-kernel@vger.kernel.org, lse-tech@lists.sourceforge.net
Subject: Re: [Lse-tech] [Patch 6/8] delay accounting usage of taskstats interface
References: <20060502061930.GC22607@in.ibm.com>
In-Reply-To: <20060502061930.GC22607@in.ibm.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2576
Lines: 88

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/