Re: [patch 1/3] add basic accounting fields to taskstats

[Balbir Singh <balbir@in.ibm.com>]

Shailabh Nagar wrote:
> Jay Lan wrote:
>> Balbir Singh wrote:
>>
>>> Jay Lan wrote:
>>>
>>>>  
>>>> -#define TASKSTATS_VERSION    1
>>>> +#define TASKSTATS_VERSION    2
>>>> +#define TASK_COMM_LEN        16
>>>>  
>>>
>>>
>>>
>>> We should find a way to keep this in sync with with the definition
>>> in linux/sched.h (won't we a warning if both this header and
>>> linux/sched.h are included together?)
>>
>>
>> I do not know how to sync it up. This header linux/taskstats.h is
>> meant to be included by userspace programs. If an application
>> happens to include linux/sched.h, which includes linux/time.h,
>> the application will very likely have compilation errors because
>> the "struct timespec" declaration in <linux/time.h> and <time.h>
>> are conflicting.
>>
>> The <linux/acct.h> defines it to
>> #define ACCT_COMM    16
>>
>> I can change our define to TS_COMM_LEN with remakes saying it
>> should be in sync with the TAKS_COMM_LEN defined in linux/sched.h.
> 
> This seems like a good enough way to do it. There's no real need for
> the taskstats comm length to remain exactly in sync with the task struct's
> comm length (by way of trying to include sched.h etc.) though avoiding the
> compile error by renaming is desirable as Balbir pointed out.
> 
> Moreover, TASK_COMM_LEN in linux/sched.h isn't likely to change much -
> if it increases and csa_acct users also really need the extra info 
> provided,
> taskstats can always be changed and version bumped up. If the size 
> decreases
> there's no harm done (strncpy should be sufficient protection).
> 
> --Shailabh
> 

I am not sure if there is a version of BUG_ON() for compile time
asserts. Basically, if we have an infrastructure of the form

/*
  * From C/C++ users journal November 2004
  */
#define STATIC_BUG_ON(e) 	\
	switch (0) {		\
	case  0:		\
	case (e):		\
		;		\
	}

Then the STATIC_BUG_ON() can catch as shown below.

#define TASK_COMM_LEN 	16
#define T_COMM_LEN	20

int
main(void)
{
	STATIC_BUG_ON(TASK_COMM_LEN == T_COMM_LEN);
}

STATIC_BUG_ON gives the following warning

bug_on_c.c: In function `main':
bug_on_c.c:19: duplicate case value
bug_on_c.c:19: previously used here

but with T_COMM_LEN set to 16

It compiles without any errors, the code generated also
looks like it has no overhead

int
main(void)
{
  8048310:       55                      push   %ebp
  8048311:       89 e5                   mov    %esp,%ebp
  8048313:       83 ec 08                sub    $0x8,%esp
  8048316:       83 e4 f0                and    $0xfffffff0,%esp
         STATIC_BUG_ON(TASK_COMM_LEN == T_COMM_LEN);
}
  8048319:       c9                      leave
  804831a:       c3                      ret
  804831b:       90                      nop


Assuming such infrastructure is available, you could then
do

#ifdef __KERNEL__
#include <linux/sched.h>
#define TS_COMM_LEN	16
STATIC_BUG_ON (TS_COMM_LEN == TASK_COMM_LEN);
#endif

Comments?

-- 

	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs