On Tue, Apr 27, 2021 at 10:25:31AM -0400, Brian Foster wrote: > On Tue, Apr 27, 2021 at 09:13:18PM +0800, Gao Xiang wrote: > > On Tue, Apr 27, 2021 at 08:46:25AM -0400, Brian Foster wrote: > > > On Tue, Apr 27, 2021 at 09:12:01AM +0800, Gao Xiang wrote: > > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > > > Keep the mount superblock counters up to date for !lazysbcount > > > > filesystems so that when we log the superblock they do not need > > > > updating in any way because they are already correct. > > > > > > > > It's found by what Zorro reported: > > > > 1. mkfs.xfs -f -l lazy-count=0 -m crc=0 $dev > > > > 2. mount $dev $mnt > > > > 3. fsstress -d $mnt -p 100 -n 1000 (maybe need more or less io load) > > > > 4. umount $mnt > > > > 5. xfs_repair -n $dev > > > > and I've seen no problem with this patch. > > > > > > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > > > > Reported-by: Zorro Lang <zlang@xxxxxxxxxx> > > > > Reviewed-by: Gao Xiang <hsiangkao@xxxxxxxxxx> > > > > Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx> > > > > --- > > > > > > Could you provide a bit more detail on the problem in the commit log? > > > From the description and code change, it seems like there is some > > > problem with doing the percpu aggregation in xfs_log_sb() on > > > !lazysbcount filesystems. Therefore this patch reserves that behavior > > > for lazysbcount, and instead enables per-transaction updates in the > > > !lazysbcount specific cleanup path. Am I following that correctly? > > > > This patch inherited from Dave's patch [1] (and I added reproduable > > steps), > > https://lore.kernel.org/r/20210422014446.GZ63242@xxxxxxxxxxxxxxxxxxx > > > > More details see my original patch v2: > > https://lore.kernel.org/r/20210420110855.2961626-1-hsiangkao@xxxxxxxxxx > > > > Ok, thanks. So the bit about xfs_log_sb() is to avoid an incorrect > overwrite of the in-core sb counters from the percpu counters on > !lazysbcount. The xfs_trans_apply_sb_deltas() function already applies > the transaction deltas to the on-disk superblock buffer, so the change > to xfs_trans_unreserve_and_mod_sb() is basically to apply those same > deltas to the in-core superblock so they are consistent in the > !lazysbcount case... yes? If I'm following that correctly, this looks > good to me: Yeah, that's right :) > > Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx> Thanks for the review! Thanks, Gao Xiang > > > Thanks, > > Gao Xiang > > > > > > > > Brian
