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

[Long Li <leo.lilong@huawei.com>]

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.