Re: [Lse-tech] [PATCH 00/11] Task watchers: Introduction

[Peter Williams <pwil3058@bigpond.net.au>]

Matt Helsley wrote:
> On Thu, 2006-06-22 at 14:26 +1000, Peter Williams wrote:
> 
> <snip>
> 
>>>>>> BTW as a former user of PAGG, I think there are ideas in the PAGG 
>>>>>> implementation that you should look at.  In particular:
>>>>>>
>>>>>> 1. The use of an array of function pointers (one for each hook) can cut 
>>>>>> down on the overhead.  The notifier_block only needs to contain a 
>>>>>> pointer to the array so there's no increase in the size of that 
>>>>>> structure.  Within the array a null pointer would mean "don't bother 
>>>>>> calling".  Only one real array needs to exist even for per task as 
>>>>>> they're all using the same functions (just separate data).  It removes 
>>>>>> the need for a switch statement in the client's function as well as 
>>>>>> saving on unnecessary function calls.
>>>>> 	I don't think having an explicit array of function pointers is likely
>>>>> to be as fast as a switch statement (or a simple branch) generated by
>>>>> the compiler.
>>>> With the array there's no need for any switch or branching.  You know 
>>>> exactly which function in the array to use in each hook.
>>> 	I don't forsee enough of a difference to make this worth arguing about.
>>> You're welcome to benchmark and compare arrays vs. switches/branches on
>>> a variety of archs, SMP boxen, NUMA boxen, etc. and post the results.
>>> I'm going to focus on other issues for now.
>>>
>>>>> 	It doesn't save unecessary function calls unless I modify the core
>>>>> notifier block structure. Otherwise I still need to stuff a generic
>>>>> function into .notifier_call and from there get the pointer to the array
>>>>> to make the next call. So it adds more pointer indirection but does not
>>>>> reduce the number of intermediate function calls.
>>>> There comes a point when trying to reuse existing code is less cost 
>>>> effective than starting over.
>>> Write my own notifier chains just to avoid a function call? I don't
>>> think that's sufficient justification for implementing my own.
>> Can't help thinking why the easier option of adding setuid and setgid 
>> hooks to PAGG and then including PAGG wasn't adopted.
> 
> 	Task watchers is not intended to group tasks. It's intended to factor a
> common pattern out of these paths in a way useful to existing parts of
> the kernel, proposed changes, and modules.
> 
> 	Your goal of grouping tasks seems like it could use task watchers. That
> does not mean that every task watcher needs to manage groups of tasks.

The same is true of PAGG (in spite of the implication in its name). 
It's really just a general task tracking and call back mechanism (with 
an ability to store per task data in a way that is easy to find from a 
call back -- just like per task watchers) and grouping is just one of 
things it can be used for.  A lot of work went into making it safe and 
relatively easy to use from modules.  It's a pity to see all that work 
go to waste.

Admittedly, it didn't have hooks for setuid and setgid but that would 
have been easy to fix.  Easier than getting task watchers to the same 
level of maturity and ease of use.  Of course, the ease of use issues 
won't bite until somebody tries to do something substantial with task 
watchers from a loadable module as (once you get the hang of them) task 
watchers are quite easy to use from inside the kernel.

A lot of work went to make the call back mechanisms in PAGG efficient as 
well but (like you) I don't think that's a very big issue as the hooks 
aren't on a busy path.

Peter
PS A year or so ago the CKRM folks promised to have a look at using PAGG 
instead of inventing their own but I doubt that they ever did.
-- 
Peter Williams                                   pwil3058@bigpond.net.au

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce