Message ID | 20210727062053.11129-6-allison.henderson@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Delayed Attributes | expand |
On 27 Jul 2021 at 11:50, Allison Henderson wrote: > This is a quick patch to add a new xfs_attr_*_return tracepoints. We > use these to track when ever a new state is set or -EAGAIN is returned > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/libxfs/xfs_attr.c | 28 ++++++++++++++++++++++++++-- > fs/xfs/libxfs/xfs_attr_remote.c | 1 + > fs/xfs/xfs_trace.h | 24 ++++++++++++++++++++++++ > 3 files changed, 51 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 5040fc1..b0c6c62 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -335,6 +335,7 @@ xfs_attr_sf_addname( > * the attr fork to leaf format and will restart with the leaf > * add. > */ > + trace_xfs_attr_sf_addname_return(XFS_DAS_UNINIT, args->dp); > dac->flags |= XFS_DAC_DEFER_FINISH; > return -EAGAIN; > } > @@ -394,6 +395,8 @@ xfs_attr_set_iter( > * handling code below > */ > dac->flags |= XFS_DAC_DEFER_FINISH; > + trace_xfs_attr_set_iter_return( > + dac->dela_state, args->dp); > return -EAGAIN; > } else if (error) { > return error; > @@ -418,6 +421,7 @@ xfs_attr_set_iter( > > dac->dela_state = XFS_DAS_FOUND_NBLK; > } > + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); > return -EAGAIN; > case XFS_DAS_FOUND_LBLK: > /* > @@ -445,6 +449,8 @@ xfs_attr_set_iter( > error = xfs_attr_rmtval_set_blk(dac); > if (error) > return error; > + trace_xfs_attr_set_iter_return(dac->dela_state, > + args->dp); > return -EAGAIN; > } > > @@ -479,6 +485,7 @@ xfs_attr_set_iter( > * series. > */ > dac->dela_state = XFS_DAS_FLIP_LFLAG; > + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); > return -EAGAIN; > case XFS_DAS_FLIP_LFLAG: > /* > @@ -496,6 +503,9 @@ xfs_attr_set_iter( > dac->dela_state = XFS_DAS_RM_LBLK; > if (args->rmtblkno) { > error = __xfs_attr_rmtval_remove(dac); > + if (error == -EAGAIN) > + trace_xfs_attr_set_iter_return( > + dac->dela_state, args->dp); > if (error) > return error; if __xfs_attr_rmtval_remove() successfully removes all the remote blocks, we transition over to XFS_DAS_RD_LEAF state and return -EAGAIN. A tracepoint is probably required for this as well? > > @@ -549,6 +559,8 @@ xfs_attr_set_iter( > error = xfs_attr_rmtval_set_blk(dac); > if (error) > return error; > + trace_xfs_attr_set_iter_return( > + dac->dela_state, args->dp); > return -EAGAIN; > } > > @@ -584,6 +596,7 @@ xfs_attr_set_iter( > * series > */ > dac->dela_state = XFS_DAS_FLIP_NFLAG; > + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); > return -EAGAIN; > > case XFS_DAS_FLIP_NFLAG: > @@ -603,6 +616,10 @@ xfs_attr_set_iter( > dac->dela_state = XFS_DAS_RM_NBLK; > if (args->rmtblkno) { > error = __xfs_attr_rmtval_remove(dac); > + if (error == -EAGAIN) > + trace_xfs_attr_set_iter_return( > + dac->dela_state, args->dp); > + > if (error) > return error; A transition to XFS_DAS_CLR_FLAG state probably requires a tracepoint call. > > @@ -1183,6 +1200,8 @@ xfs_attr_node_addname( > * this point. > */ > dac->flags |= XFS_DAC_DEFER_FINISH; > + trace_xfs_attr_node_addname_return( > + dac->dela_state, args->dp); > return -EAGAIN; > } > > @@ -1429,10 +1448,13 @@ xfs_attr_remove_iter( > * blocks are removed. > */ > error = __xfs_attr_rmtval_remove(dac); > - if (error == -EAGAIN) > + if (error == -EAGAIN) { > + trace_xfs_attr_remove_iter_return( > + dac->dela_state, args->dp); > return error; > - else if (error) > + } else if (error) { > goto out; > + } > if the call to __xfs_attr_rmtval_remove() is successful, we transition over to XFS_DAS_RM_NAME state and return -EAGAIN. Maybe tracepoint is required here as well? > /* > * Refill the state structure with buffers (the prior > @@ -1473,6 +1495,8 @@ xfs_attr_remove_iter( > > dac->flags |= XFS_DAC_DEFER_FINISH; > dac->dela_state = XFS_DAS_RM_SHRINK; > + trace_xfs_attr_remove_iter_return( > + dac->dela_state, args->dp); > return -EAGAIN; > } > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c > index 0c8bee3..70f880d 100644 > --- a/fs/xfs/libxfs/xfs_attr_remote.c > +++ b/fs/xfs/libxfs/xfs_attr_remote.c > @@ -696,6 +696,7 @@ __xfs_attr_rmtval_remove( > */ > if (!done) { > dac->flags |= XFS_DAC_DEFER_FINISH; > + trace_xfs_attr_rmtval_remove_return(dac->dela_state, args->dp); > return -EAGAIN; > } > > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h > index f9d8d60..f9840dd 100644 > --- a/fs/xfs/xfs_trace.h > +++ b/fs/xfs/xfs_trace.h > @@ -3987,6 +3987,30 @@ DEFINE_ICLOG_EVENT(xlog_iclog_want_sync); > DEFINE_ICLOG_EVENT(xlog_iclog_wait_on); > DEFINE_ICLOG_EVENT(xlog_iclog_write); > > +DECLARE_EVENT_CLASS(xfs_das_state_class, > + TP_PROTO(int das, struct xfs_inode *ip), > + TP_ARGS(das, ip), > + TP_STRUCT__entry( > + __field(int, das) > + __field(xfs_ino_t, ino) > + ), > + TP_fast_assign( > + __entry->das = das; > + __entry->ino = ip->i_ino; > + ), > + TP_printk("state change %d ino 0x%llx", > + __entry->das, __entry->ino) > +) > + > +#define DEFINE_DAS_STATE_EVENT(name) \ > +DEFINE_EVENT(xfs_das_state_class, name, \ > + TP_PROTO(int das, struct xfs_inode *ip), \ > + TP_ARGS(das, ip)) > +DEFINE_DAS_STATE_EVENT(xfs_attr_sf_addname_return); > +DEFINE_DAS_STATE_EVENT(xfs_attr_set_iter_return); > +DEFINE_DAS_STATE_EVENT(xfs_attr_node_addname_return); > +DEFINE_DAS_STATE_EVENT(xfs_attr_remove_iter_return); > +DEFINE_DAS_STATE_EVENT(xfs_attr_rmtval_remove_return); > #endif /* _TRACE_XFS_H */ > > #undef TRACE_INCLUDE_PATH
On 7/28/21 6:42 AM, Chandan Babu R wrote: > On 27 Jul 2021 at 11:50, Allison Henderson wrote: >> This is a quick patch to add a new xfs_attr_*_return tracepoints. We >> use these to track when ever a new state is set or -EAGAIN is returned >> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >> Reviewed-by: Darrick J. Wong <djwong@kernel.org> >> --- >> fs/xfs/libxfs/xfs_attr.c | 28 ++++++++++++++++++++++++++-- >> fs/xfs/libxfs/xfs_attr_remote.c | 1 + >> fs/xfs/xfs_trace.h | 24 ++++++++++++++++++++++++ >> 3 files changed, 51 insertions(+), 2 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index 5040fc1..b0c6c62 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -335,6 +335,7 @@ xfs_attr_sf_addname( >> * the attr fork to leaf format and will restart with the leaf >> * add. >> */ >> + trace_xfs_attr_sf_addname_return(XFS_DAS_UNINIT, args->dp); >> dac->flags |= XFS_DAC_DEFER_FINISH; >> return -EAGAIN; >> } >> @@ -394,6 +395,8 @@ xfs_attr_set_iter( >> * handling code below >> */ >> dac->flags |= XFS_DAC_DEFER_FINISH; >> + trace_xfs_attr_set_iter_return( >> + dac->dela_state, args->dp); >> return -EAGAIN; >> } else if (error) { >> return error; >> @@ -418,6 +421,7 @@ xfs_attr_set_iter( >> >> dac->dela_state = XFS_DAS_FOUND_NBLK; >> } >> + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); >> return -EAGAIN; >> case XFS_DAS_FOUND_LBLK: >> /* >> @@ -445,6 +449,8 @@ xfs_attr_set_iter( >> error = xfs_attr_rmtval_set_blk(dac); >> if (error) >> return error; >> + trace_xfs_attr_set_iter_return(dac->dela_state, >> + args->dp); >> return -EAGAIN; >> } >> >> @@ -479,6 +485,7 @@ xfs_attr_set_iter( >> * series. >> */ >> dac->dela_state = XFS_DAS_FLIP_LFLAG; >> + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); >> return -EAGAIN; >> case XFS_DAS_FLIP_LFLAG: >> /* >> @@ -496,6 +503,9 @@ xfs_attr_set_iter( >> dac->dela_state = XFS_DAS_RM_LBLK; >> if (args->rmtblkno) { >> error = __xfs_attr_rmtval_remove(dac); >> + if (error == -EAGAIN) >> + trace_xfs_attr_set_iter_return( >> + dac->dela_state, args->dp); >> if (error) >> return error; > > if __xfs_attr_rmtval_remove() successfully removes all the remote blocks, we > transition over to XFS_DAS_RD_LEAF state and return -EAGAIN. A tracepoint > is probably required for this as well? > >> >> @@ -549,6 +559,8 @@ xfs_attr_set_iter( >> error = xfs_attr_rmtval_set_blk(dac); >> if (error) >> return error; >> + trace_xfs_attr_set_iter_return( >> + dac->dela_state, args->dp); >> return -EAGAIN; >> } >> >> @@ -584,6 +596,7 @@ xfs_attr_set_iter( >> * series >> */ >> dac->dela_state = XFS_DAS_FLIP_NFLAG; >> + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); >> return -EAGAIN; >> >> case XFS_DAS_FLIP_NFLAG: >> @@ -603,6 +616,10 @@ xfs_attr_set_iter( >> dac->dela_state = XFS_DAS_RM_NBLK; >> if (args->rmtblkno) { >> error = __xfs_attr_rmtval_remove(dac); >> + if (error == -EAGAIN) >> + trace_xfs_attr_set_iter_return( >> + dac->dela_state, args->dp); >> + >> if (error) >> return error; > > A transition to XFS_DAS_CLR_FLAG state probably requires a tracepoint call. > >> >> @@ -1183,6 +1200,8 @@ xfs_attr_node_addname( >> * this point. >> */ >> dac->flags |= XFS_DAC_DEFER_FINISH; >> + trace_xfs_attr_node_addname_return( >> + dac->dela_state, args->dp); >> return -EAGAIN; >> } >> >> @@ -1429,10 +1448,13 @@ xfs_attr_remove_iter( >> * blocks are removed. >> */ >> error = __xfs_attr_rmtval_remove(dac); >> - if (error == -EAGAIN) >> + if (error == -EAGAIN) { >> + trace_xfs_attr_remove_iter_return( >> + dac->dela_state, args->dp); >> return error; >> - else if (error) >> + } else if (error) { >> goto out; >> + } >> > > if the call to __xfs_attr_rmtval_remove() is successful, we transition over to > XFS_DAS_RM_NAME state and return -EAGAIN. Maybe tracepoint is required here as > well? Yes, I think these three are good additions. I think this patch was written before those extra states were added, and likley forgot to add the corresponding traces. Will update. Thanks for the review! Allison > >> /* >> * Refill the state structure with buffers (the prior >> @@ -1473,6 +1495,8 @@ xfs_attr_remove_iter( >> >> dac->flags |= XFS_DAC_DEFER_FINISH; >> dac->dela_state = XFS_DAS_RM_SHRINK; >> + trace_xfs_attr_remove_iter_return( >> + dac->dela_state, args->dp); >> return -EAGAIN; >> } >> >> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c >> index 0c8bee3..70f880d 100644 >> --- a/fs/xfs/libxfs/xfs_attr_remote.c >> +++ b/fs/xfs/libxfs/xfs_attr_remote.c >> @@ -696,6 +696,7 @@ __xfs_attr_rmtval_remove( >> */ >> if (!done) { >> dac->flags |= XFS_DAC_DEFER_FINISH; >> + trace_xfs_attr_rmtval_remove_return(dac->dela_state, args->dp); >> return -EAGAIN; >> } >> >> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h >> index f9d8d60..f9840dd 100644 >> --- a/fs/xfs/xfs_trace.h >> +++ b/fs/xfs/xfs_trace.h >> @@ -3987,6 +3987,30 @@ DEFINE_ICLOG_EVENT(xlog_iclog_want_sync); >> DEFINE_ICLOG_EVENT(xlog_iclog_wait_on); >> DEFINE_ICLOG_EVENT(xlog_iclog_write); >> >> +DECLARE_EVENT_CLASS(xfs_das_state_class, >> + TP_PROTO(int das, struct xfs_inode *ip), >> + TP_ARGS(das, ip), >> + TP_STRUCT__entry( >> + __field(int, das) >> + __field(xfs_ino_t, ino) >> + ), >> + TP_fast_assign( >> + __entry->das = das; >> + __entry->ino = ip->i_ino; >> + ), >> + TP_printk("state change %d ino 0x%llx", >> + __entry->das, __entry->ino) >> +) >> + >> +#define DEFINE_DAS_STATE_EVENT(name) \ >> +DEFINE_EVENT(xfs_das_state_class, name, \ >> + TP_PROTO(int das, struct xfs_inode *ip), \ >> + TP_ARGS(das, ip)) >> +DEFINE_DAS_STATE_EVENT(xfs_attr_sf_addname_return); >> +DEFINE_DAS_STATE_EVENT(xfs_attr_set_iter_return); >> +DEFINE_DAS_STATE_EVENT(xfs_attr_node_addname_return); >> +DEFINE_DAS_STATE_EVENT(xfs_attr_remove_iter_return); >> +DEFINE_DAS_STATE_EVENT(xfs_attr_rmtval_remove_return); >> #endif /* _TRACE_XFS_H */ >> >> #undef TRACE_INCLUDE_PATH > >
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 5040fc1..b0c6c62 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -335,6 +335,7 @@ xfs_attr_sf_addname( * the attr fork to leaf format and will restart with the leaf * add. */ + trace_xfs_attr_sf_addname_return(XFS_DAS_UNINIT, args->dp); dac->flags |= XFS_DAC_DEFER_FINISH; return -EAGAIN; } @@ -394,6 +395,8 @@ xfs_attr_set_iter( * handling code below */ dac->flags |= XFS_DAC_DEFER_FINISH; + trace_xfs_attr_set_iter_return( + dac->dela_state, args->dp); return -EAGAIN; } else if (error) { return error; @@ -418,6 +421,7 @@ xfs_attr_set_iter( dac->dela_state = XFS_DAS_FOUND_NBLK; } + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); return -EAGAIN; case XFS_DAS_FOUND_LBLK: /* @@ -445,6 +449,8 @@ xfs_attr_set_iter( error = xfs_attr_rmtval_set_blk(dac); if (error) return error; + trace_xfs_attr_set_iter_return(dac->dela_state, + args->dp); return -EAGAIN; } @@ -479,6 +485,7 @@ xfs_attr_set_iter( * series. */ dac->dela_state = XFS_DAS_FLIP_LFLAG; + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); return -EAGAIN; case XFS_DAS_FLIP_LFLAG: /* @@ -496,6 +503,9 @@ xfs_attr_set_iter( dac->dela_state = XFS_DAS_RM_LBLK; if (args->rmtblkno) { error = __xfs_attr_rmtval_remove(dac); + if (error == -EAGAIN) + trace_xfs_attr_set_iter_return( + dac->dela_state, args->dp); if (error) return error; @@ -549,6 +559,8 @@ xfs_attr_set_iter( error = xfs_attr_rmtval_set_blk(dac); if (error) return error; + trace_xfs_attr_set_iter_return( + dac->dela_state, args->dp); return -EAGAIN; } @@ -584,6 +596,7 @@ xfs_attr_set_iter( * series */ dac->dela_state = XFS_DAS_FLIP_NFLAG; + trace_xfs_attr_set_iter_return(dac->dela_state, args->dp); return -EAGAIN; case XFS_DAS_FLIP_NFLAG: @@ -603,6 +616,10 @@ xfs_attr_set_iter( dac->dela_state = XFS_DAS_RM_NBLK; if (args->rmtblkno) { error = __xfs_attr_rmtval_remove(dac); + if (error == -EAGAIN) + trace_xfs_attr_set_iter_return( + dac->dela_state, args->dp); + if (error) return error; @@ -1183,6 +1200,8 @@ xfs_attr_node_addname( * this point. */ dac->flags |= XFS_DAC_DEFER_FINISH; + trace_xfs_attr_node_addname_return( + dac->dela_state, args->dp); return -EAGAIN; } @@ -1429,10 +1448,13 @@ xfs_attr_remove_iter( * blocks are removed. */ error = __xfs_attr_rmtval_remove(dac); - if (error == -EAGAIN) + if (error == -EAGAIN) { + trace_xfs_attr_remove_iter_return( + dac->dela_state, args->dp); return error; - else if (error) + } else if (error) { goto out; + } /* * Refill the state structure with buffers (the prior @@ -1473,6 +1495,8 @@ xfs_attr_remove_iter( dac->flags |= XFS_DAC_DEFER_FINISH; dac->dela_state = XFS_DAS_RM_SHRINK; + trace_xfs_attr_remove_iter_return( + dac->dela_state, args->dp); return -EAGAIN; } diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c index 0c8bee3..70f880d 100644 --- a/fs/xfs/libxfs/xfs_attr_remote.c +++ b/fs/xfs/libxfs/xfs_attr_remote.c @@ -696,6 +696,7 @@ __xfs_attr_rmtval_remove( */ if (!done) { dac->flags |= XFS_DAC_DEFER_FINISH; + trace_xfs_attr_rmtval_remove_return(dac->dela_state, args->dp); return -EAGAIN; } diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index f9d8d60..f9840dd 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -3987,6 +3987,30 @@ DEFINE_ICLOG_EVENT(xlog_iclog_want_sync); DEFINE_ICLOG_EVENT(xlog_iclog_wait_on); DEFINE_ICLOG_EVENT(xlog_iclog_write); +DECLARE_EVENT_CLASS(xfs_das_state_class, + TP_PROTO(int das, struct xfs_inode *ip), + TP_ARGS(das, ip), + TP_STRUCT__entry( + __field(int, das) + __field(xfs_ino_t, ino) + ), + TP_fast_assign( + __entry->das = das; + __entry->ino = ip->i_ino; + ), + TP_printk("state change %d ino 0x%llx", + __entry->das, __entry->ino) +) + +#define DEFINE_DAS_STATE_EVENT(name) \ +DEFINE_EVENT(xfs_das_state_class, name, \ + TP_PROTO(int das, struct xfs_inode *ip), \ + TP_ARGS(das, ip)) +DEFINE_DAS_STATE_EVENT(xfs_attr_sf_addname_return); +DEFINE_DAS_STATE_EVENT(xfs_attr_set_iter_return); +DEFINE_DAS_STATE_EVENT(xfs_attr_node_addname_return); +DEFINE_DAS_STATE_EVENT(xfs_attr_remove_iter_return); +DEFINE_DAS_STATE_EVENT(xfs_attr_rmtval_remove_return); #endif /* _TRACE_XFS_H */ #undef TRACE_INCLUDE_PATH