Messages in this thread | | | Subject | Re: 2.6.18-rc2-mm1 | From | Matt Helsley <> | Date | Fri, 28 Jul 2006 11:49:27 -0700 |
| |
On Fri, 2006-07-28 at 01:34 -0700, Andrew Morton wrote: > On Fri, 28 Jul 2006 10:17:44 +0200 > "Michal Piotrowski" <michal.k.k.piotrowski@gmail.com> wrote: > > > Matt, can you look at this? > > > > My hunt file shows me, that this patches are causing oops. > > GOOD > > # > > # > > task-watchers-task-watchers.patch > > task-watchers-register-process-events-task-watcher.patch > > task-watchers-refactor-process-events.patch > > task-watchers-make-process-events-configurable-as.patch > > task-watchers-allow-task-watchers-to-block.patch > > task-watchers-register-audit-task-watcher.patch > > task-watchers-register-per-task-delay-accounting.patch > > task-watchers-register-profile-as-a-task-watcher.patch > > task-watchers-add-support-for-per-task-watchers.patch > > task-watchers-register-semundo-task-watcher.patch > > task-watchers-register-per-task-semundo-watcher.patch > > BAD > > Thanks for working that out.
I noticed the delay accounting functions in the stack trace. Perhaps task-watchers-register-per-task-delay-accounting.patch is causing the problem. With all of the recent churn in per-task delay accounting I'm wondering if that patch is still correct. Balbir, Shailabh, what do you think?
> I've actually been thinking that we shouldn't proceed with those patches. > > They're a nice cleanup and make the kernel code _look_ better and I really > like them because of this. But the cost is potentially significant. We > replace N direct calls with a walk of a notifier chain, more than N > indirect calls, demultiplexing at the other end and then a direct call. > That's a significant amount of additional overhead to make the kernel > source look nicer.
OK. The multiple notifier chain approach you suggested gets rid of demultiplexing. We'd replace N direct calls with a walk of a notifier chain and (more than??) N indirect calls.
An alternative suggested to me by Al Viro is to handle these functions much like the *_initcall() macros in include/linux/init.h. This replaces N direct calls with an array walk and N indirect calls. Unfortunately, this does not work for modules.
> Plus, ugly though it is, you can look at the current code and see what it's > doing. With a notifier chain you have to grep around the tree and work out > what might be hooking into the chain, which is harder.
The same is true when using function pointers in other areas of the kernel. They make the code harder to trace but have advantages you can't get by simply pasting the function call into the path.
That said, I think walking the code is a bit easier with the multichain approach. Lastly, at each invocation I could put in a comment explaining how to find users of the chain.
> Finally, the consolidation into a notifier chain forces all the > fork/exit/exec hooks into an one-size-fits-all model. What happens if one > subsystem wants to hook in before exit_mmap() and another one wants to hook > in after exit_mmap() (for example)?
As I see it there's not much I can do about the one-size-fits-all model. So I tried to find the one size that fits most.
I think many systems that place calls in these paths aren't as sensitive to their precise location as you might imagine. Many need to initialize their per-task data and clean it up. They tend to depend on a valid task structure and little else. Fewer systems -- profile for instance -- have very specific requirements for when/where they get called. Yet even profile can use some of task watchers.
I considered some obvious alternatives but they had worse problems. Adding more notifications will run into naming problems. Using notifier block priorities would have similar problems and be even harder to trace by hand.
For those special systems that don't fit this "size" I think leaving them in these paths is the best approach.
Cheers, -Matt Helsley
- 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/
| |