Re: [patch 2/3] add CSA accounting to taskstats

[Jay Lan <jlan@sgi.com>]

Pavel Machek wrote:
> Hi!

Hi Pavel,

Thanks much for your feedback.

However, the patchset you commented on was replaced by a new
patchset posted on 8/03:
http://marc.theaimsgroup.com/?l=linux-kernel&m=115457888801994&w=2

Thanks,
  - jay

> 
> 
>>Signed-off-by:  Jay Lan <jlan@sgi.com>
>>
> 
>>Index: linux/include/linux/taskstats.h
>>===================================================================
>>--- linux.orig/include/linux/taskstats.h	2006-07-31 11:42:10.000000000 -0700
>>+++ linux/include/linux/taskstats.h	2006-07-31 11:50:00.412433042 -0700
>>@@ -107,6 +107,21 @@ struct taskstats {
>> 	__u64	ac_utime;		/* User CPU time [usec] */
>> 	__u64	ac_stime;		/* SYstem CPU time [usec] */
>> 	/* Basic Accounting Fields end */
>>+
>>+ 	/* CSA accounting fields start */
>>+ 	__u16	csa_revision;		/* CSA Revision */
>>+ 	__u16	csa_pad[3];		/* Unused */
> 
> 
> I guess you have way too many TLAs here...
> 
> 
>>+config CSA_ACCT
>>+	bool "Enable CSA Job Accounting (EXPERIMENTAL)"
>>+	depends on TASKSTATS
>>+	help
> 
> 
> "Enable Comprehensive System Accounting Job Accounting" . Ouch. So you
> do not even know how to use those accronyms correctly.
> 
> I guess you should invent some better naming.
> 
> 
> 
>>+	  Comprehensive System Accounting (CSA) provides job level
>>+	  accounting of resource usage.  The accounting records are
>>+	  written by the kernel into a file.  CSA user level scripts
>>+	  and commands process the binary accounting records and
>>+	  combine them by job identifier within system boot uptime
>>+	  periods.  These accounting records are then used to produce
>>+	  reports and charge fees to users.
>>+
>>+	  Say Y here if you want job level accounting to be compiled
>>+	  into the kernel.  Say M here if you want the writing of
>>+	  accounting records portion of this feature to be a loadable
>>+	  module.  Say N here if you do not want job level accounting
>>+	  (the default).
>>+
>>+	  The CSA commands and scripts package needs to be installed
>>+	  to process the CSA accounting records.  See
>>+	  http://oss.sgi.com/projects/csa for further information
>>+	  about CSA and download instructions for the CSA commands
>>+	  package and documentation.
> 
> 
> ...long text and it *still* does not tell me what it is good for.
> 
> 
>>+/*
>>+ * Record revision levels.
>>+ *
>>+ * These are incremented to indicate that a record's format has changed since
>>+ * a previous release.
>>+ *
>>+ * History:     05000   The first rev in Linux
>>+ *              06000   Major rework to clean up unused fields and features.
>>+ *                      No binary compatibility with earlier rev.
>>+ *		07000	Convert to taskstats interface
>>+ */
>>+#define REV_CSA		07000	/* Kernel: CSA base record */
> 
> 
> We normally drop back compatibility at merge...
> 									Pavel
>