Steve Dickson wrote:
> 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 <jmo...@redhat.com>
>>
>> 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);
Probably should be before the wait because if it expires away we
probably won't be able to retrieve the path.
>>
>> 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?
I suspect that's likely not going to be the most interesting case for
us. People are likely to want to know when mounting and expiring is
happening but and know when a fail return occurrs, of course, that still
leaves the issue of being swamped with output.
>
>> 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
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs
