| Message ID | 20200304075401.21558-7-david@fromorbit.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: clean up log tickets and record writes | expand |
On Wed, Mar 04, 2020 at 06:53:56PM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > To clean up unmount record writing error handling, we need to move > xlog_state_ioerror() higher up in the file. Also move the setting of > the XLOG_IO_ERROR state to inside the function. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> FYI, I have a pending series that kills xlog_state_ioerror and XLOG_IO_ERROR. Let me send that out now that Brians fix is in for-next.
On Wed, Mar 04, 2020 at 07:51:40AM -0800, Christoph Hellwig wrote: > On Wed, Mar 04, 2020 at 06:53:56PM +1100, Dave Chinner wrote: > > From: Dave Chinner <dchinner@redhat.com> > > > > To clean up unmount record writing error handling, we need to move > > xlog_state_ioerror() higher up in the file. Also move the setting of > > the XLOG_IO_ERROR state to inside the function. > > > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > > FYI, I have a pending series that kills xlog_state_ioerror and > XLOG_IO_ERROR. Let me send that out now that Brians fix is in for-next. Can you rebase that on top of this? removing IOERROR is a much more invasive and riskier set of state machine changes compared to what this patchset does. This patchset simplifies some of the error handling that handles IOERROR, too... Cheers, Dave.
On Thu, Mar 05, 2020 at 08:41:15AM +1100, Dave Chinner wrote: > On Wed, Mar 04, 2020 at 07:51:40AM -0800, Christoph Hellwig wrote: > > On Wed, Mar 04, 2020 at 06:53:56PM +1100, Dave Chinner wrote: > > > From: Dave Chinner <dchinner@redhat.com> > > > > > > To clean up unmount record writing error handling, we need to move > > > xlog_state_ioerror() higher up in the file. Also move the setting of > > > the XLOG_IO_ERROR state to inside the function. > > > > > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > > > > FYI, I have a pending series that kills xlog_state_ioerror and > > XLOG_IO_ERROR. Let me send that out now that Brians fix is in for-next. > > Can you rebase that on top of this? removing IOERROR is a much more > invasive and riskier set of state machine changes compared to what > this patchset does. This patchset simplifies some of the error > handling that handles IOERROR, too... I find this lipstick on the pig here a little pointless, especially moving things around that are planned to be removed..
On Wed, Mar 04, 2020 at 06:53:56PM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > To clean up unmount record writing error handling, we need to move > xlog_state_ioerror() higher up in the file. Also move the setting of > the XLOG_IO_ERROR state to inside the function. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > --- > fs/xfs/xfs_log.c | 59 ++++++++++++++++++++++++------------------------ > 1 file changed, 30 insertions(+), 29 deletions(-) > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index 2e9f3baa7cc8..0de3c32d42b6 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -874,6 +874,36 @@ xfs_log_mount_cancel( > xfs_log_unmount(mp); > } > > +/* > + * Mark all iclogs IOERROR. l_icloglock is held by the caller. > + */ > +STATIC int > +xlog_state_ioerror( > + struct xlog *log) > +{ > + xlog_in_core_t *iclog, *ic; Might as well kill off the typedef usage, though it sounds like Christoph has code to remove this. Eh, that discussion aside this looks fine to me: Reviewed-by: Brian Foster <bfoster@redhat.com> > + > + log->l_flags |= XLOG_IO_ERROR; > + > + iclog = log->l_iclog; > + if (iclog->ic_state != XLOG_STATE_IOERROR) { > + /* > + * Mark all the incore logs IOERROR. > + * From now on, no log flushes will result. > + */ > + ic = iclog; > + do { > + ic->ic_state = XLOG_STATE_IOERROR; > + ic = ic->ic_next; > + } while (ic != iclog); > + return 0; > + } > + /* > + * Return non-zero, if state transition has already happened. > + */ > + return 1; > +} > + > /* > * Mark the filesystem clean by writing an unmount record to the head of the > * log. > @@ -3770,34 +3800,6 @@ xlog_verify_iclog( > } /* xlog_verify_iclog */ > #endif > > -/* > - * Mark all iclogs IOERROR. l_icloglock is held by the caller. > - */ > -STATIC int > -xlog_state_ioerror( > - struct xlog *log) > -{ > - xlog_in_core_t *iclog, *ic; > - > - iclog = log->l_iclog; > - if (iclog->ic_state != XLOG_STATE_IOERROR) { > - /* > - * Mark all the incore logs IOERROR. > - * From now on, no log flushes will result. > - */ > - ic = iclog; > - do { > - ic->ic_state = XLOG_STATE_IOERROR; > - ic = ic->ic_next; > - } while (ic != iclog); > - return 0; > - } > - /* > - * Return non-zero, if state transition has already happened. > - */ > - return 1; > -} > - > /* > * This is called from xfs_force_shutdown, when we're forcibly > * shutting down the filesystem, typically because of an IO error. > @@ -3868,7 +3870,6 @@ xfs_log_force_umount( > * Mark the log and the iclogs with IO error flags to prevent any > * further log IO from being issued or completed. > */ > - log->l_flags |= XLOG_IO_ERROR; > retval = xlog_state_ioerror(log); > spin_unlock(&log->l_icloglock); > > -- > 2.24.0.rc0 >
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index 2e9f3baa7cc8..0de3c32d42b6 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -874,6 +874,36 @@ xfs_log_mount_cancel( xfs_log_unmount(mp); } +/* + * Mark all iclogs IOERROR. l_icloglock is held by the caller. + */ +STATIC int +xlog_state_ioerror( + struct xlog *log) +{ + xlog_in_core_t *iclog, *ic; + + log->l_flags |= XLOG_IO_ERROR; + + iclog = log->l_iclog; + if (iclog->ic_state != XLOG_STATE_IOERROR) { + /* + * Mark all the incore logs IOERROR. + * From now on, no log flushes will result. + */ + ic = iclog; + do { + ic->ic_state = XLOG_STATE_IOERROR; + ic = ic->ic_next; + } while (ic != iclog); + return 0; + } + /* + * Return non-zero, if state transition has already happened. + */ + return 1; +} + /* * Mark the filesystem clean by writing an unmount record to the head of the * log. @@ -3770,34 +3800,6 @@ xlog_verify_iclog( } /* xlog_verify_iclog */ #endif -/* - * Mark all iclogs IOERROR. l_icloglock is held by the caller. - */ -STATIC int -xlog_state_ioerror( - struct xlog *log) -{ - xlog_in_core_t *iclog, *ic; - - iclog = log->l_iclog; - if (iclog->ic_state != XLOG_STATE_IOERROR) { - /* - * Mark all the incore logs IOERROR. - * From now on, no log flushes will result. - */ - ic = iclog; - do { - ic->ic_state = XLOG_STATE_IOERROR; - ic = ic->ic_next; - } while (ic != iclog); - return 0; - } - /* - * Return non-zero, if state transition has already happened. - */ - return 1; -} - /* * This is called from xfs_force_shutdown, when we're forcibly * shutting down the filesystem, typically because of an IO error. @@ -3868,7 +3870,6 @@ xfs_log_force_umount( * Mark the log and the iclogs with IO error flags to prevent any * further log IO from being issued or completed. */ - log->l_flags |= XLOG_IO_ERROR; retval = xlog_state_ioerror(log); spin_unlock(&log->l_icloglock);