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

[Matt Helsley <matthltc@us.ibm.com>]

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.

Cheers,
	-Matt Helsley