Re: Patch 2/9] Initialization

[Shailabh Nagar <nagar@watson.ibm.com>]

Jes Sorensen wrote:

> > > > > > "Shailabh" == Shailabh Nagar <nagar@watson.ibm.com> writes:
> > > > > >            
>
> Shailabh> delayacct-setup.patch Initialization code related to
> Shailabh> collection of per-task "delay" statistics which measure how
> Shailabh> long it had to wait for cpu, sync block io, swapping
> Shailabh> etc. The collection of statistics and the interface are in
> Shailabh> other patches. This patch sets up the data structures and
> Shailabh> allows the statistics collection to be disabled through a
> Shailabh> kernel boot paramater.
>
> Shailabh> +#ifdef CONFIG_TASK_DELAY_ACCT
> Shailabh> +struct task_delay_info {
> Shailabh> +	spinlock_t	lock;
> Shailabh> +
> Shailabh> +	/* For each stat XXX, add following, aligned appropriately
> Shailabh> +	 *
> Shailabh> +	 * struct timespec XXX_start, XXX_end;
> Shailabh> +	 * u64 XXX_delay;
> Shailabh> +	 * u32 XXX_count;
> Shailabh> +	 */
> Shailabh> +};
> Shailabh> +#endif
>
> Hmmm
>
> I thought you were going to change this to do
>
> u64 some_delay
> u32 foo_count
> u32 bar_count
> u64 another_delay
>
> To avoid wasting space on 64 bit platforms.
>  

Well, the "aligned appropriately" part of the comment was intended to let
future users know that something like the above should be done.

e.g in a subsequent patch (5/9)
http://www.uwsg.indiana.edu/hypermail/linux/kernel/0603.1/1919.html
when we introduce the additional stat swapin_delay/count, a 64-bit friendly
alignment is being done thus:

u64 blkio_delay; /* wait for sync block io completion */
+ u64 swapin_delay; /* wait for sync block io completion */
u32 blkio_count;
+ u32 swapin_count;

The need for each stat, potentially, to need a separate set of timespec 
variables
to reduce nesting problems does introduce another wrinkle in 
cache-friendliness
since you'd ideally like to have all the XXX_* variables close together. 
Right now
we're good since swapin doesn't need extra timespecs of its own. But future
additions might need to be more careful.

--Shailabh


> Jes
>  

-
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/