Re: [PATCH] filename in tapset nfs.proc.open and nfs.proc.release

[David Smith <dsmith@redhat.com>]

On 12/13/2014 10:35 PM, hai wu wrote:
> tapset nfs.proc.open and nfs.proc.release in nfs_proc.stp only records
> file's basename, which is not helpful when we use it to monitor NFS
> file access activities.
> 
> The following patch would allow us to be able to see the full path for
> NFS file, along with its NFS mount information.
> 
> diff --git a/tapset/linux/nfs_proc.stp b/tapset/linux/nfs_proc.stp
> index 1339aee..5a804e4 100644
> --- a/tapset/linux/nfs_proc.stp
> +++ b/tapset/linux/nfs_proc.stp
> @@ -1658,7 +1658,7 @@ probe nfs.proc.open = kernel.function("nfs_open") !,
>         prot = get_prot_from_client(client)
>         version = __nfs_version($inode)
> 
> -       filename = __file_filename($filp)
> +  filename = task_dentry_path(task_current(), $filp->f_dentry, $filp->f_vfsmnt)

Thanks for the patch, but I don't think this is really what we want
here. Here's why:

- The __file_filename() function just does a few casts and copies a
field out of $filp. It is fairly low cost.

- The task_dentry_path() function has to recursively walk the filesystem
building the full path. It can be quite expensive, depending on how deep
the file is in the nfs mount or how far down the the nfs mount is
mounted on the client system.

In general, we try to keep those convenience variables low cost. If your
use case needs a full path, that's fine. We just shouldn't make everyone
pay the higher cost.


Your 2nd email noted that you needed a different line for RHEL6:

filename = task_dentry_path(task_current(), $filp->f_path->dentry,
$filp->f_path->mnt)

Here's a (untested) way to use the same source line everywhere:

  pathname = task_dentry_path(task_current(),
	@choose_defined($filp->f_dentry, $filp->f_path->dentry),
	@choose_defined($filp->f_vfsmnt, $filp->f_path->mnt))

I hope this helps.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)