Re: [PATCH 09/11] xfs: merge unmount record write iclog cleanup.

Christoph Hellwig
2020-03-05T15:27:34Z

On Thu, Mar 05, 2020 at 08:38:54AM +1100, Dave Chinner wrote:
> On Wed, Mar 04, 2020 at 07:53:32AM -0800, Christoph Hellwig wrote:
> > On Wed, Mar 04, 2020 at 06:53:59PM +1100, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > >
> > > The unmount iclog handling is duplicated in both
> > > xfs_log_unmount_write() and xfs_log_write_unmount_record(). We only
> > > need one copy of it in xfs_log_unmount_write() because that is the
> > > only function that calls xfs_log_write_unmount_record().
> >
> > The copy in xfs_log_unmount_write actually is dead code. It only
> > is called in the XLOG_FORCED_SHUTDOWN case, in which case all iclogs
> > are marked as STATE_IOERROR, and thus xlog_state_release_iclog is
> > a no-op. I really need to send the series out to clean this up
> > ASAP..
>
> Well, this patch pretty much solves that "dead code" problem in that
> it now handles already shut down, error in unmount record write and
> successful unmount record write now. i.e. we run the same code in
> all cases now, so you'll only need to fix the IOERROR handling in
> one place :P

It doesn't really. We still end up with more convoluted logic after
your series, that goes through a whole bunch of steps that don't make
any sense when the log is already shut down. Just like everywhere
else we should just return early with a shutdown log / iclog and just
delete this code instead of rearranging the chairs.