Re: [Patch 1/4] Delay accounting: Initialization

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

Parag Warudkar wrote:
> 
> On Nov 15, 2005, at 5:29 PM, Shailabh Nagar wrote:
<snip>
> 
>>> Does this mean, whether or not the per task delay accounting is used,
>>> we have a constant overhead of sizeof(spinlock_t) + 2*sizeof  (uint32_t)
>>> + 2* sizeof(uint64_t) bytes going into the struct  task_struct?.  Is it
>>> possible/beneficial to use struct task_delay_info  *delays instead  and
>>> allocate it if task wants to use the information?
>>>
>>
>> Doing so would have value in the case where the feature is  configured
>> but no one ever registers to listen for it.
> 
> 
> Precisely. Such a feature will be used only occasionally I suppose.
> I didn't read the code deeply but are any scheduling decisions  altered
> based on this data? If not, then it makes sense to not account unless required.
> 
> I think it should be possible to do it on demand, per process instead 
> of forcing the accounting on _all_ processes which cumulatively becomes a  sizeable
> o/h.
> 
> Per Process activation of this feature will add significant value  IMHO.
> (Of course, if that's possible in first place.)


Per-task activation is useful/possible only for long-running tasks.
If one is trying to gather stats for a user-defined grouping of tasks
then it would involve too much overhead & inaccuracy to require monitoring
to be turned on individually.

> 
>> The cost of doing this would be
>> - adding more code to the fork path to allocate conditionally
> 
> 
> Just an unlikely branch for normal code path - not a big deal.
> Also I am thinking it could be handled outside of fork?

only if per-task activation is done - thats probably what you meant ?

> 
>> - make the collecting of the delays conditional on a similar check
> 
> 
> Weighing this against the actual accounting - I think it's a win.

Hmmm..since there is locking involved in the stats collection, this is
starting to make a lot of sense.

> 
>> - cache pollution from following an extra pointer in the pgflt/
>> io_schedule paths
>> I'm not sure is this really matters for these two code paths.
> 
> 
> Possibly.
> 
>> Even if one does this, once the first listener registers, all  future
>> tasks
>> (and even the current ones) will have to go ahead and allocate the 
>> structure
>> and accounting of delays will have to switch to unconditional mode. 
>> This is
>> because the delay data has cumulative value...future listeners will be
>> interested in data collected earlier (as long as task is still 
>> running). And
>> once the first listener registers, you can no longer be sure no  one's
>> interested
>> in the future.
>>
> 
> Is it possible to do it per process? Forcing it on all processes is 
> what I was trying to avoid given the feature's usage pattern.
> 
>> Another alternative is to let userland control the overhead of 
>> allocation and
>> collection completely through a /proc/sys/kernel/delayacct variable.
>> When its switched on, it triggers an allocation for all existing 
>> tasks in the
>> system, turns on allocation in fork() for future tasks, and 
>> collection of the stats.
>> When turned off, collection of stats stops as does allocation for 
>> future tasks
>> (not worth going in and deallocating structs for existing tasks).
> 
> 
>> Does this seem worth it ?
>>
> 
> Definitely not unless we can do it per process and on demand.

Per-task doesn't seem like a good idea. Neither does allowing dynamic
switching off/on of the allocation of the delay struct.

So how about this:

Have /proc/sys/kernel/delayacct and a corresponding kernel boot parameter (for setting
the switch early) which control just the collection of data. Allocation always happens.


>> -- Shailabh
>>
> 
> Cheers
> 
> Parag
>