On Mon, Oct 31, 2022 at 09:04:41AM +1100, Dave Chinner wrote:
> On Sat, Oct 29, 2022 at 03:16:01PM +0800, Long Li wrote:
> > On Thu, Oct 27, 2022 at 09:05:21AM -0700, Darrick J. Wong wrote:
> > > On Thu, Oct 27, 2022 at 09:25:04PM +0800, Long Li wrote:
> > > > not pass, therefore it will not write a clean umount record
> > > > at umount. I also haven't found a code suitable for adding
> > > > such checks.
> > >
> > > xfs_unmountfs just prior to unmounting the log.
> >
> >
> > I tried to add an extra check in xfs_log_unmount_write, when m_icount <
> > m_ifree, it will not write a umount log record, after which the summary
> > counters will be recalculated at next mount. If m_ifree greater than
> > m_icount in memory, sb_i{count,free} (the ondisk superblock inode counters)
> > maybe incorrect even after unmount filesystem. After adding such checks,
> > it can be corrected on the next mount, instead of going undetected in
> > subsequent mounts.
> >
> > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > index f1f44c006ab3..e4903c15019e 100644
> > --- a/fs/xfs/xfs_log.c
> > +++ b/fs/xfs/xfs_log.c
> > @@ -1038,7 +1038,9 @@ xfs_log_unmount_write(
> > * more details.
> > */
> > if (XFS_TEST_ERROR(xfs_fs_has_sickness(mp, XFS_SICK_FS_COUNTERS), mp,
> > - XFS_ERRTAG_FORCE_SUMMARY_RECALC)) {
> > + XFS_ERRTAG_FORCE_SUMMARY_RECALC) ||
> > + (percpu_counter_sum(&mp->m_icount) <
> > + percpu_counter_sum(&mp->m_ifree))) {
> > xfs_alert(mp, "%s: will fix summary counters at next mount",
> > __func__);
> > return;
>
> The log code is not the layer at which the mount structures
> should be verified. xfs_unmountfs() is where the mount is cleaned up
> and all activity is flushed and waited on. THis is where the mount
> counters should be checked, before we unmount the log.
>
> Indeed, if you check the mount counters prior to calling
> xfs_log_unmount_write(), you could call this:
>
> xfs_alert(mp, "ifree/icount mismatch at unmount");
> xfs_fs_mark_sick(mp, XFS_SICK_FS_COUNTERS);
>
> i.e. check the mount state at the correct level and propagate the
> sickness into the mount state and the log code will just do the
> right thing....
>
> Cheers,
>
> Dave.
Ok, I'll resend a patch and fix the above issue, thanks a lot.
