From: Jeff Layton <jlayton@kernel.org>
To: David Howells <dhowells@redhat.com>, linux-cachefs@redhat.com
Cc: Trond Myklebust <trondmy@hammerspace.com>,
Anna Schumaker <anna.schumaker@netapp.com>,
Steve French <smfrench@gmail.com>,
Dominique Martinet <asmadeus@codewreck.org>,
Matthew Wilcox <willy@infradead.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Omar Sandoval <osandov@osandov.com>,
JeffleXu <jefflexu@linux.alibaba.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
linux-afs@lists.infradead.org, linux-nfs@vger.kernel.org,
linux-cifs@vger.kernel.org, ceph-devel@vger.kernel.org,
v9fs-developer@lists.sourceforge.net,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 04/11] cachefiles: Make some tracepoint adjustments
Date: Fri, 21 Jan 2022 12:52:27 -0500 [thread overview]
Message-ID: <49f163f8c073756346f486a916e568e8744f3be0.camel@kernel.org> (raw)
In-Reply-To: <164251403694.3435901.9797725381831316715.stgit@warthog.procyon.org.uk>
On Tue, 2022-01-18 at 13:53 +0000, David Howells wrote:
> Make some adjustments to tracepoints to make the tracing a bit more
> followable:
>
> (1) Standardise on displaying the backing inode number as "B=<hex>" with
> no leading zeros.
>
> (2) Make the cachefiles_lookup tracepoint log the directory inode number
> as well as the looked-up inode number.
>
> (3) Add a cachefiles_lookup tracepoint into cachefiles_get_directory() to
> log directory lookup.
>
> (4) Add a new cachefiles_mkdir tracepoint and use that to log a successful
> mkdir from cachefiles_get_directory().
>
> (5) Make the cachefiles_unlink and cachefiles_rename tracepoints log the
> inode number of the affected file/dir rather than dentry struct
> pointers.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-cachefs@redhat.com
> ---
>
> fs/cachefiles/namei.c | 8 ++--
> include/trace/events/cachefiles.h | 82 +++++++++++++++++++++++--------------
> 2 files changed, 56 insertions(+), 34 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index 9bd692870617..52c9f0864a87 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -101,6 +101,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
> subdir = lookup_one_len(dirname, dir, strlen(dirname));
> else
> subdir = ERR_PTR(ret);
> + trace_cachefiles_lookup(NULL, dir, subdir);
> if (IS_ERR(subdir)) {
> trace_cachefiles_vfs_error(NULL, d_backing_inode(dir),
> PTR_ERR(subdir),
> @@ -135,6 +136,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
> cachefiles_trace_mkdir_error);
> goto mkdir_error;
> }
> + trace_cachefiles_mkdir(dir, subdir);
>
> if (unlikely(d_unhashed(subdir))) {
> cachefiles_put_directory(subdir);
> @@ -233,7 +235,7 @@ static int cachefiles_unlink(struct cachefiles_cache *cache,
> };
> int ret;
>
> - trace_cachefiles_unlink(object, dentry, why);
> + trace_cachefiles_unlink(object, d_inode(dentry)->i_ino, why);
> ret = security_path_unlink(&path, dentry);
> if (ret < 0) {
> cachefiles_io_error(cache, "Unlink security error");
> @@ -386,7 +388,7 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> .new_dir = d_inode(cache->graveyard),
> .new_dentry = grave,
> };
> - trace_cachefiles_rename(object, rep, grave, why);
> + trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
> ret = cachefiles_inject_read_error();
> if (ret == 0)
> ret = vfs_rename(&rd);
> @@ -617,7 +619,7 @@ bool cachefiles_look_up_object(struct cachefiles_object *object)
> object->d_name_len);
> else
> dentry = ERR_PTR(ret);
> - trace_cachefiles_lookup(object, dentry);
> + trace_cachefiles_lookup(object, fan, dentry);
> if (IS_ERR(dentry)) {
> if (dentry == ERR_PTR(-ENOENT))
> goto new_file;
> diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
> index 1172529b5b49..093c4acb7a3a 100644
> --- a/include/trace/events/cachefiles.h
> +++ b/include/trace/events/cachefiles.h
> @@ -233,25 +233,48 @@ TRACE_EVENT(cachefiles_ref,
>
> TRACE_EVENT(cachefiles_lookup,
> TP_PROTO(struct cachefiles_object *obj,
> + struct dentry *dir,
> struct dentry *de),
>
> - TP_ARGS(obj, de),
> + TP_ARGS(obj, dir, de),
>
> TP_STRUCT__entry(
> __field(unsigned int, obj )
> __field(short, error )
> + __field(unsigned long, dino )
> __field(unsigned long, ino )
> ),
>
> TP_fast_assign(
> - __entry->obj = obj->debug_id;
> + __entry->obj = obj ? obj->debug_id : 0;
> + __entry->dino = d_backing_inode(dir)->i_ino;
> __entry->ino = (!IS_ERR(de) && d_backing_inode(de) ?
> d_backing_inode(de)->i_ino : 0);
> __entry->error = IS_ERR(de) ? PTR_ERR(de) : 0;
> ),
>
> - TP_printk("o=%08x i=%lx e=%d",
> - __entry->obj, __entry->ino, __entry->error)
> + TP_printk("o=%08x dB=%lx B=%lx e=%d",
> + __entry->obj, __entry->dino, __entry->ino, __entry->error)
> + );
> +
> +TRACE_EVENT(cachefiles_mkdir,
> + TP_PROTO(struct dentry *dir, struct dentry *subdir),
> +
> + TP_ARGS(dir, subdir),
> +
> + TP_STRUCT__entry(
> + __field(unsigned int, dir )
> + __field(unsigned int, subdir )
> + ),
> +
> + TP_fast_assign(
> + __entry->dir = d_backing_inode(dir)->i_ino;
> + __entry->subdir = d_backing_inode(subdir)->i_ino;
> + ),
> +
> + TP_printk("dB=%x sB=%x",
> + __entry->dir,
> + __entry->subdir)
> );
>
> TRACE_EVENT(cachefiles_tmpfile,
> @@ -269,7 +292,7 @@ TRACE_EVENT(cachefiles_tmpfile,
> __entry->backer = backer->i_ino;
> ),
>
> - TP_printk("o=%08x b=%08x",
> + TP_printk("o=%08x B=%x",
> __entry->obj,
> __entry->backer)
> );
> @@ -289,61 +312,58 @@ TRACE_EVENT(cachefiles_link,
> __entry->backer = backer->i_ino;
> ),
>
> - TP_printk("o=%08x b=%08x",
> + TP_printk("o=%08x B=%x",
> __entry->obj,
> __entry->backer)
> );
>
> TRACE_EVENT(cachefiles_unlink,
> TP_PROTO(struct cachefiles_object *obj,
> - struct dentry *de,
> + ino_t ino,
> enum fscache_why_object_killed why),
>
> - TP_ARGS(obj, de, why),
> + TP_ARGS(obj, ino, why),
>
> /* Note that obj may be NULL */
> TP_STRUCT__entry(
> __field(unsigned int, obj )
> - __field(struct dentry *, de )
> + __field(unsigned int, ino )
> __field(enum fscache_why_object_killed, why )
> ),
>
> TP_fast_assign(
> __entry->obj = obj ? obj->debug_id : UINT_MAX;
> - __entry->de = de;
> + __entry->ino = ino;
> __entry->why = why;
> ),
>
> - TP_printk("o=%08x d=%p w=%s",
> - __entry->obj, __entry->de,
> + TP_printk("o=%08x B=%x w=%s",
> + __entry->obj, __entry->ino,
> __print_symbolic(__entry->why, cachefiles_obj_kill_traces))
> );
>
> TRACE_EVENT(cachefiles_rename,
> TP_PROTO(struct cachefiles_object *obj,
> - struct dentry *de,
> - struct dentry *to,
> + ino_t ino,
> enum fscache_why_object_killed why),
>
> - TP_ARGS(obj, de, to, why),
> + TP_ARGS(obj, ino, why),
>
> /* Note that obj may be NULL */
> TP_STRUCT__entry(
> __field(unsigned int, obj )
> - __field(struct dentry *, de )
> - __field(struct dentry *, to )
> + __field(unsigned int, ino )
> __field(enum fscache_why_object_killed, why )
> ),
>
> TP_fast_assign(
> __entry->obj = obj ? obj->debug_id : UINT_MAX;
> - __entry->de = de;
> - __entry->to = to;
> + __entry->ino = ino;
> __entry->why = why;
> ),
>
> - TP_printk("o=%08x d=%p t=%p w=%s",
> - __entry->obj, __entry->de, __entry->to,
> + TP_printk("o=%08x B=%x w=%s",
> + __entry->obj, __entry->ino,
> __print_symbolic(__entry->why, cachefiles_obj_kill_traces))
> );
>
> @@ -370,7 +390,7 @@ TRACE_EVENT(cachefiles_coherency,
> __entry->ino = ino;
> ),
>
> - TP_printk("o=%08x %s i=%llx c=%u",
> + TP_printk("o=%08x %s B=%llx c=%u",
> __entry->obj,
> __print_symbolic(__entry->why, cachefiles_coherency_traces),
> __entry->ino,
> @@ -397,7 +417,7 @@ TRACE_EVENT(cachefiles_vol_coherency,
> __entry->ino = ino;
> ),
>
> - TP_printk("V=%08x %s i=%llx",
> + TP_printk("V=%08x %s B=%llx",
> __entry->vol,
> __print_symbolic(__entry->why, cachefiles_coherency_traces),
> __entry->ino)
> @@ -435,7 +455,7 @@ TRACE_EVENT(cachefiles_prep_read,
> __entry->cache_inode = cache_inode;
> ),
>
> - TP_printk("R=%08x[%u] %s %s f=%02x s=%llx %zx ni=%x b=%x",
> + TP_printk("R=%08x[%u] %s %s f=%02x s=%llx %zx ni=%x B=%x",
> __entry->rreq, __entry->index,
> __print_symbolic(__entry->source, netfs_sreq_sources),
> __print_symbolic(__entry->why, cachefiles_prepare_read_traces),
> @@ -466,7 +486,7 @@ TRACE_EVENT(cachefiles_read,
> __entry->len = len;
> ),
>
> - TP_printk("o=%08x b=%08x s=%llx l=%zx",
> + TP_printk("o=%08x B=%x s=%llx l=%zx",
> __entry->obj,
> __entry->backer,
> __entry->start,
> @@ -495,7 +515,7 @@ TRACE_EVENT(cachefiles_write,
> __entry->len = len;
> ),
>
> - TP_printk("o=%08x b=%08x s=%llx l=%zx",
> + TP_printk("o=%08x B=%x s=%llx l=%zx",
> __entry->obj,
> __entry->backer,
> __entry->start,
> @@ -524,7 +544,7 @@ TRACE_EVENT(cachefiles_trunc,
> __entry->why = why;
> ),
>
> - TP_printk("o=%08x b=%08x %s l=%llx->%llx",
> + TP_printk("o=%08x B=%x %s l=%llx->%llx",
> __entry->obj,
> __entry->backer,
> __print_symbolic(__entry->why, cachefiles_trunc_traces),
> @@ -549,7 +569,7 @@ TRACE_EVENT(cachefiles_mark_active,
> __entry->inode = inode->i_ino;
> ),
>
> - TP_printk("o=%08x i=%lx",
> + TP_printk("o=%08x B=%lx",
> __entry->obj, __entry->inode)
> );
>
> @@ -570,7 +590,7 @@ TRACE_EVENT(cachefiles_mark_inactive,
> __entry->inode = inode->i_ino;
> ),
>
> - TP_printk("o=%08x i=%lx",
> + TP_printk("o=%08x B=%lx",
> __entry->obj, __entry->inode)
> );
>
> @@ -594,7 +614,7 @@ TRACE_EVENT(cachefiles_vfs_error,
> __entry->where = where;
> ),
>
> - TP_printk("o=%08x b=%08x %s e=%d",
> + TP_printk("o=%08x B=%x %s e=%d",
> __entry->obj,
> __entry->backer,
> __print_symbolic(__entry->where, cachefiles_error_traces),
> @@ -621,7 +641,7 @@ TRACE_EVENT(cachefiles_io_error,
> __entry->where = where;
> ),
>
> - TP_printk("o=%08x b=%08x %s e=%d",
> + TP_printk("o=%08x B=%x %s e=%d",
> __entry->obj,
> __entry->backer,
> __print_symbolic(__entry->where, cachefiles_error_traces),
>
>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2022-01-21 17:52 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
2022-01-18 13:53 ` [PATCH 01/11] fscache: Fix the volume collision wait condition David Howells
2022-01-18 13:53 ` [PATCH 02/11] cachefiles: Calculate the blockshift in terms of bytes, not pages David Howells
2022-01-21 17:47 ` Jeff Layton
2022-01-18 13:53 ` [PATCH 03/11] cachefiles: set default tag name if it's unspecified David Howells
2022-01-21 17:51 ` Jeff Layton
2022-01-18 13:53 ` [PATCH 04/11] cachefiles: Make some tracepoint adjustments David Howells
2022-01-21 17:52 ` Jeff Layton [this message]
2022-01-18 13:54 ` [PATCH 05/11] cachefiles: Trace active-mark failure David Howells
2022-01-21 17:53 ` Jeff Layton
2022-01-18 13:54 ` [PATCH 06/11] cachefiles: Explain checks in a comment David Howells
2022-01-18 13:54 ` [PATCH 07/11] cachefiles: Check that the backing filesystem supports tmpfiles David Howells
2022-01-18 13:54 ` [PATCH 08/11] fscache: Add a comment explaining how page-release optimisation works David Howells
2022-01-18 13:54 ` [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag David Howells
2022-01-18 16:23 ` Christoph Hellwig
2022-01-18 17:40 ` David Howells
2022-01-19 5:20 ` Christoph Hellwig
2022-01-19 9:18 ` David Howells
2022-01-19 11:15 ` Christian Brauner
2022-01-20 9:08 ` Christoph Hellwig
2022-01-20 9:37 ` David Howells
2022-01-18 13:55 ` [PATCH 10/11] netfs: Make ops->init_rreq() optional David Howells
2022-01-18 13:55 ` [PATCH 11/11] cifs: Support fscache indexing rewrite David Howells
2022-01-19 5:44 ` Steve French
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49f163f8c073756346f486a916e568e8744f3be0.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=anna.schumaker@netapp.com \
--cc=asmadeus@codewreck.org \
--cc=ceph-devel@vger.kernel.org \
--cc=dhowells@redhat.com \
--cc=jefflexu@linux.alibaba.com \
--cc=linux-afs@lists.infradead.org \
--cc=linux-cachefs@redhat.com \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=osandov@osandov.com \
--cc=smfrench@gmail.com \
--cc=torvalds@linux-foundation.org \
--cc=trondmy@hammerspace.com \
--cc=v9fs-developer@lists.sourceforge.net \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome