On Mon, 2021-06-21 at 22:47 +0100, David Howells wrote:
> fscache_cookie_put() accesses the cookie it has just put inside the
> tracepoint that monitors the change - but this is something it's not
> allowed to do if we didn't reduce the count to zero.
Do you mean "if the count went to zero." ?
>
> Fix this by dropping most of those values from the tracepoint and grabbing
> the cookie debug ID before doing the dec.
>
> Also take the opportunity to switch over the usage and where arguments on
> the tracepoint to put the reason last.
>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> ---
>
> fs/fscache/cookie.c | 10 ++++++----
> fs/fscache/internal.h | 2 +-
> fs/fscache/netfs.c | 2 +-
> include/trace/events/fscache.h | 24 +++++++-----------------
> 4 files changed, 15 insertions(+), 23 deletions(-)
>
> diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
> index 2558814193e9..6df3732cf1b4 100644
> --- a/fs/fscache/cookie.c
> +++ b/fs/fscache/cookie.c
> @@ -225,8 +225,8 @@ struct fscache_cookie *fscache_hash_cookie(struct fscache_cookie *candidate)
>
> collision:
> if (test_and_set_bit(FSCACHE_COOKIE_ACQUIRED, &cursor->flags)) {
> - trace_fscache_cookie(cursor, fscache_cookie_collision,
> - atomic_read(&cursor->usage));
> + trace_fscache_cookie(cursor->debug_id, atomic_read(&cursor->usage),
> + fscache_cookie_collision);
> pr_err("Duplicate cookie detected\n");
> fscache_print_cookie(cursor, 'O');
> fscache_print_cookie(candidate, 'N');
> @@ -305,7 +305,8 @@ struct fscache_cookie *__fscache_acquire_cookie(
>
> cookie = fscache_hash_cookie(candidate);
> if (!cookie) {
> - trace_fscache_cookie(candidate, fscache_cookie_discard, 1);
> + trace_fscache_cookie(candidate->debug_id, 1,
> + fscache_cookie_discard);
> goto out;
> }
>
> @@ -866,8 +867,9 @@ void fscache_cookie_put(struct fscache_cookie *cookie,
> _enter("%x", cookie->debug_id);
>
> do {
> + unsigned int cookie_debug_id = cookie->debug_id;
> usage = atomic_dec_return(&cookie->usage);
> - trace_fscache_cookie(cookie, where, usage);
> + trace_fscache_cookie(cookie_debug_id, usage, where);
>
> if (usage > 0)
> return;
> diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
> index a49136c63e4b..345105dbbfd1 100644
> --- a/fs/fscache/internal.h
> +++ b/fs/fscache/internal.h
> @@ -291,7 +291,7 @@ static inline void fscache_cookie_get(struct fscache_cookie *cookie,
> {
> int usage = atomic_inc_return(&cookie->usage);
>
> - trace_fscache_cookie(cookie, where, usage);
> + trace_fscache_cookie(cookie->debug_id, usage, where);
> }
>
> /*
> diff --git a/fs/fscache/netfs.c b/fs/fscache/netfs.c
> index cce92216fa28..d6bdb7b5e723 100644
> --- a/fs/fscache/netfs.c
> +++ b/fs/fscache/netfs.c
> @@ -37,7 +37,7 @@ int __fscache_register_netfs(struct fscache_netfs *netfs)
> if (!cookie)
> goto already_registered;
> if (cookie != candidate) {
> - trace_fscache_cookie(candidate, fscache_cookie_discard, 1);
> + trace_fscache_cookie(candidate->debug_id, 1, fscache_cookie_discard);
> fscache_free_cookie(candidate);
> }
>
> diff --git a/include/trace/events/fscache.h b/include/trace/events/fscache.h
> index 0b9e058aba4d..55b8802740fa 100644
> --- a/include/trace/events/fscache.h
> +++ b/include/trace/events/fscache.h
> @@ -160,37 +160,27 @@ fscache_cookie_traces;
>
>
> TRACE_EVENT(fscache_cookie,
> - TP_PROTO(struct fscache_cookie *cookie,
> - enum fscache_cookie_trace where,
> - int usage),
> + TP_PROTO(unsigned int cookie_debug_id,
> + int usage,
> + enum fscache_cookie_trace where),
>
> - TP_ARGS(cookie, where, usage),
> + TP_ARGS(cookie_debug_id, usage, where),
>
> TP_STRUCT__entry(
> __field(unsigned int, cookie )
> - __field(unsigned int, parent )
> __field(enum fscache_cookie_trace, where )
> __field(int, usage )
> - __field(int, n_children )
> - __field(int, n_active )
> - __field(u8, flags )
> ),
>
> TP_fast_assign(
> - __entry->cookie = cookie->debug_id;
> - __entry->parent = cookie->parent ? cookie->parent->debug_id : 0;
> + __entry->cookie = cookie_debug_id;
> __entry->where = where;
> __entry->usage = usage;
> - __entry->n_children = atomic_read(&cookie->n_children);
> - __entry->n_active = atomic_read(&cookie->n_active);
> - __entry->flags = cookie->flags;
> ),
>
> - TP_printk("%s c=%08x u=%d p=%08x Nc=%d Na=%d f=%02x",
> + TP_printk("%s c=%08x u=%d",
> __print_symbolic(__entry->where, fscache_cookie_traces),
> - __entry->cookie, __entry->usage,
> - __entry->parent, __entry->n_children, __entry->n_active,
> - __entry->flags)
> + __entry->cookie, __entry->usage)
> );
>
> TRACE_EVENT(fscache_netfs,
>
>
Patch itself looks fine though.
--
Jeff Layton <jlayton@xxxxxxxxxx>
