Re: [Lse-tech] [PATCH 00/11] Task watchers: Introduction
From: Peter Williams
Date: Thu Jun 22 2006 - 02:28:31 EST
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@xxxxxxxxxxxxxx
"Learning, n. The kind of ignorance distinguishing the studious."
-- Ambrose Bierce
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/