Re: [PATCH v2] xfs: fix sb write verify for lazysbcount

[Dave Chinner <david@fromorbit.com>]

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.

-- 
Dave Chinner
david@fromorbit.com