Hey, Comments in-line...
On 06/30/2009 01:08 PM, Jeff Moyer wrote: > Hi, Ian, > > I got a fairly simply request from a customer. They wanted to use > systemtap to track automounter mount and unmount activity. > Unfortunately, the compiler inlined the expire functions I wanted to > instrument, so I needed to add a tracepoint to get at useful > information. > > This patch, then, adds a few trace points so that we don't have to worry > about compiler inlining. I have an example script using these trace > points that I'll be sending along to the systemtap mailing list > shortly. Let me know what you think. If you're interested in trying it > out, I've made my systemtap patch available at: > http://people.redhat.com/jmoyer/systemtap > > You can try running testsuite/systemtap.examples/general/autofs.stp. It > should run with or without the below patch applied, though the results > are not as accurate (can't get full paths for unmounts) without the > tracepoint in autofs4_do_expire_multi. > > Cheers, > Jeff > > Signed-off-by: Jeff Moyer <[email protected]> > > diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c > index aa39ae8..6c86df9 100644 > --- a/fs/autofs4/expire.c > +++ b/fs/autofs4/expire.c > @@ -13,6 +13,7 @@ > * ------------------------------------------------------------------------- > */ > > #include "autofs_i.h" > +#include <trace/events/autofs4.h> > > static unsigned long now; > > @@ -496,6 +497,7 @@ int autofs4_do_expire_multi(struct super_block *sb, > struct vfsmount *mnt, > /* This is synchronous because it makes the daemon a > little easier */ > ret = autofs4_wait(sbi, dentry, NFY_EXPIRE); > + trace_autofs4_do_expire_multi(mnt, dentry, ret); > > spin_lock(&sbi->fs_lock); > if (ino->flags & AUTOFS_INF_MOUNTPOINT) { > diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c > index b96a3c5..cbb17b7 100644 > --- a/fs/autofs4/root.c > +++ b/fs/autofs4/root.c > @@ -19,6 +19,9 @@ > #include <linux/time.h> > #include "autofs_i.h" > > +#define CREATE_TRACE_POINTS > +#include <trace/events/autofs4.h> > + > static int autofs4_dir_symlink(struct inode *,struct dentry *,const char *); > static int autofs4_dir_unlink(struct inode *,struct dentry *); > static int autofs4_dir_rmdir(struct inode *,struct dentry *); > @@ -216,6 +219,7 @@ static void *autofs4_follow_link(struct dentry *dentry, > struct nameidata *nd) > (!d_mountpoint(dentry) && __simple_empty(dentry))) { > spin_unlock(&dcache_lock); > > + trace_autofs4_follow_link(nd, dentry); > status = try_to_fill_dentry(dentry, 0); Would it make sense to put the trace point after the try_to_fill_dentry() so the status could be recorded in the trace point? Similar to the trace point above? > if (status) > goto out_error; > @@ -540,6 +544,8 @@ static struct dentry *autofs4_lookup(struct inode *dir, > struct dentry *dentry, s > dput(expiring); > } > > + trace_autofs4_lookup(nd, dentry); Same kinda idea here... would it make sense to move this trace point some where so the status can be recorded? The main point with these two question is I have found it very handy to write system tap scripts that only fire when there is a non-zero status.... Which means instead of getting pages and pages of info scrolling off the screen, I get one (or few) lines of error conditions that generally have a bit more meaning... Just a suggestion... steved. _______________________________________________ autofs mailing list [email protected] http://linux.kernel.org/mailman/listinfo/autofs
