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 <[email protected]> > --- > > 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 <[email protected]> -- Linux-cachefs mailing list [email protected] https://listman.redhat.com/mailman/listinfo/linux-cachefs
