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@xxxxxxxxxx> > > > > > > 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.