| Message ID | 20180723130414.47980-7-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: embed dfops in the transaction | expand |
On Mon, Jul 23, 2018 at 09:04:05AM -0400, Brian Foster wrote: > xfs_defer_init() is currently used in two particular situations. The > first and most obvious case is raw initialization of an > xfs_defer_ops struct. The other case is partial reinit of > xfs_defer_ops on reuse due to iteration. > > Most instances of the first case will be replaced by a single init > of a dfops embedded in the transaction. Init calls are still > technically required for the second case because the dfops may have > low space mode enabled or have joined items that need to be reset > before the dfops should be reused. > > Since the current dfops usage expects either a final transaction > commit after xfs_defer_finish() or xfs_defer_init() if dfops is to > be reused, we can shift some of the init logic into > xfs_defer_finish() such that the latter returns with a reinitialized > dfops. This eliminates the second dependency noted above such that a > dfops is immediately ready for reuse after an xfs_defer_finish() > without the need to change any calling code. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Bill O'Donnell <billodo@redhat.com> > --- > fs/xfs/libxfs/xfs_defer.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c > index 0df09c094e42..23f2a52b088e 100644 > --- a/fs/xfs/libxfs/xfs_defer.c > +++ b/fs/xfs/libxfs/xfs_defer.c > @@ -319,6 +319,19 @@ xfs_defer_bjoin( > return -EFSCORRUPTED; > } > > +/* > + * Reset an already used dfops after finish. > + */ > +static void > +xfs_defer_reset( > + struct xfs_defer_ops *dop) > +{ > + ASSERT(!xfs_defer_has_unfinished_work(dop)); > + dop->dop_low = false; > + memset(dop->dop_inodes, 0, sizeof(dop->dop_inodes)); > + memset(dop->dop_bufs, 0, sizeof(dop->dop_bufs)); > +} > + > /* > * Finish all the pending work. This involves logging intent items for > * any work items that wandered in since the last transaction roll (if > @@ -427,10 +440,13 @@ xfs_defer_finish( > dop = (*tp)->t_dfops; > } > out: > - if (error) > + if (error) { > trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error); > - else > + } else { > trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_); > + xfs_defer_reset(dop); > + } > + > return error; > } > > -- > 2.17.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 23, 2018 at 09:04:05AM -0400, Brian Foster wrote: > xfs_defer_init() is currently used in two particular situations. The > first and most obvious case is raw initialization of an > xfs_defer_ops struct. The other case is partial reinit of > xfs_defer_ops on reuse due to iteration. > > Most instances of the first case will be replaced by a single init > of a dfops embedded in the transaction. Init calls are still > technically required for the second case because the dfops may have > low space mode enabled or have joined items that need to be reset > before the dfops should be reused. > > Since the current dfops usage expects either a final transaction > commit after xfs_defer_finish() or xfs_defer_init() if dfops is to > be reused, we can shift some of the init logic into > xfs_defer_finish() such that the latter returns with a reinitialized > dfops. This eliminates the second dependency noted above such that a > dfops is immediately ready for reuse after an xfs_defer_finish() > without the need to change any calling code. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/libxfs/xfs_defer.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c > index 0df09c094e42..23f2a52b088e 100644 > --- a/fs/xfs/libxfs/xfs_defer.c > +++ b/fs/xfs/libxfs/xfs_defer.c > @@ -319,6 +319,19 @@ xfs_defer_bjoin( > return -EFSCORRUPTED; > } > > +/* > + * Reset an already used dfops after finish. > + */ > +static void > +xfs_defer_reset( > + struct xfs_defer_ops *dop) > +{ > + ASSERT(!xfs_defer_has_unfinished_work(dop)); > + dop->dop_low = false; > + memset(dop->dop_inodes, 0, sizeof(dop->dop_inodes)); > + memset(dop->dop_bufs, 0, sizeof(dop->dop_bufs)); > +} > + > /* > * Finish all the pending work. This involves logging intent items for > * any work items that wandered in since the last transaction roll (if > @@ -427,10 +440,13 @@ xfs_defer_finish( > dop = (*tp)->t_dfops; > } > out: > - if (error) > + if (error) { > trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error); > - else > + } else { > trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_); > + xfs_defer_reset(dop); > + } > + > return error; > } > > -- > 2.17.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c index 0df09c094e42..23f2a52b088e 100644 --- a/fs/xfs/libxfs/xfs_defer.c +++ b/fs/xfs/libxfs/xfs_defer.c @@ -319,6 +319,19 @@ xfs_defer_bjoin( return -EFSCORRUPTED; } +/* + * Reset an already used dfops after finish. + */ +static void +xfs_defer_reset( + struct xfs_defer_ops *dop) +{ + ASSERT(!xfs_defer_has_unfinished_work(dop)); + dop->dop_low = false; + memset(dop->dop_inodes, 0, sizeof(dop->dop_inodes)); + memset(dop->dop_bufs, 0, sizeof(dop->dop_bufs)); +} + /* * Finish all the pending work. This involves logging intent items for * any work items that wandered in since the last transaction roll (if @@ -427,10 +440,13 @@ xfs_defer_finish( dop = (*tp)->t_dfops; } out: - if (error) + if (error) { trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error); - else + } else { trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_); + xfs_defer_reset(dop); + } + return error; }