From: Matt H. <mat...@us...> - 2006-05-19 02:46:28
|
On Thu, 2006-05-18 at 09:47 +0530, Balbir Singh wrote: > > I can see how the task struct might not be sufficient. I noticed the > > taskstats functions which require more arguments are generally nearby. > > I've got an idea how to circumvent this limitation, but it's rather > > ugly: > > > > struct task_event { > > struct task_struct *task; > > union { > > struct fork {}; /* nothing extra yet */ Elaborating on this idea a bit we might have: struct fork { unsigned_long clone_flags; }; > > struct exec {}; /* nothing extra yet */ Elaborating on this idea a bit we'd have: struct exec { struct linux_binprm *bprm; }; > > struct exit { > > struct taskstats *tidstats; /* taskstats args */ > > struct taskstats *tgidstats; > > }; > > } args; > > }; > > > > fastcall NORET_TYPE void do_exit(long code) > > { > > struct task_struct *tsk = current; > > struct task_event watcher_event; > > int group_dead; > > > > watcher_event.task = tsk; > > > > ... > > > > taskstats_exit_alloc(&watcher_event.args.exit.tidstats, > > &watcher_event.args.exit.tgidstats); > > ... > > > > call_task_watchers(&watcher_event); > > > > ... > > } > > Yes, it looks a little hard-wired. For each interested notifier the size > of task_event will grow. The struct won't necessarily grow for each watcher. For instance, most watchers are interested in the task struct. I think, as Jes suggested, a good subset could use the bprm in the exec path. In the fork case I think alot of potential uses would require the clone_flags. I've shown what this might look like in my responses above. > I would suggest something more generic like > > fastcall NORET_TYPE void do_exit(long code) > { > struct task_struct *tsk = current; > struct task_event watcher_event; > int group_dead; > void *data; > > watcher_event.task = tsk; Is this structure still important to your idea? > ... > > call_task_watchers_prepare(); > ... > > call_task_watchers(); > > ... > call_task_watchers_done(); > } > > The assumption is that all the call_task_* will work with the > current task and that each notifier can register, its own private data, prepare, > done and notify functions. I don't think that get's us access to anything that couldn't already be reached from the task struct. I don't see how this would address the problem I thought we were talking about. The problem is watchers may need access to different information that is unreachable from the task struct. The different information needed is probably specific to a subset of the watchers. This problem exists no matter how many invocations of the chain occur and gets worse as we try to convert more code to use task watchers. > If required call_task_watchers_prepare() and call_task_watchers_done() > can be moved inside call_task_watchers(). Ok, you've convinced me that I have no idea what you're suggesting :). Could you elaborate on what the parameters to these calls are and where they are stored? <snip> > > I'll look into converting taskstats and/or delay accounting to be task > > watchers. > > I think this should be relatively easy to do. It will be interesting to > see the converted code. I hope so. I think taskstats is another good example of something that should be able to use these patches. <snip> Cheers, -Matt Helsley |