| Message ID | 20210126125621.3846735-4-hsiangkao@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: support shrinking free space in the last AG | expand |
On Tue, Jan 26, 2021 at 08:56:17PM +0800, Gao Xiang wrote: > sb_fdblocks will be updated lazily if lazysbcount is enabled, > therefore when shrinking the filesystem sb_fdblocks could be > larger than sb_dblocks and xfs_validate_sb_write() would fail. > > Even for growfs case, it'd be better to update lazy sb counters > immediately to reflect the real sb counters. > > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > --- > fs/xfs/xfs_fsops.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c > index a2a407039227..2e490fb75832 100644 > --- a/fs/xfs/xfs_fsops.c > +++ b/fs/xfs/xfs_fsops.c > @@ -128,6 +128,14 @@ xfs_growfs_data_private( > nb - mp->m_sb.sb_dblocks); > if (id.nfree) > xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree); > + > + /* > + * update in-core counters now to reflect the real numbers > + * (especially sb_fdblocks) > + */ Could you update the comment to explain why we do this? For example: "Sync sb counters now to reflect the updated values. This is particularly important for shrink because the write verifier will fail if sb_fdblocks is ever larger than sb_dblocks." Brian > + if (xfs_sb_version_haslazysbcount(&mp->m_sb)) > + xfs_log_sb(tp); > + > xfs_trans_set_sync(tp); > error = xfs_trans_commit(tp); > if (error) > -- > 2.27.0 >
Hi Brian, On Tue, Feb 02, 2021 at 02:38:04PM -0500, Brian Foster wrote: > On Tue, Jan 26, 2021 at 08:56:17PM +0800, Gao Xiang wrote: > > sb_fdblocks will be updated lazily if lazysbcount is enabled, > > therefore when shrinking the filesystem sb_fdblocks could be > > larger than sb_dblocks and xfs_validate_sb_write() would fail. > > > > Even for growfs case, it'd be better to update lazy sb counters > > immediately to reflect the real sb counters. > > > > Signed-off-by: Gao Xiang <hsiangkao@redhat.com> > > --- > > fs/xfs/xfs_fsops.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c > > index a2a407039227..2e490fb75832 100644 > > --- a/fs/xfs/xfs_fsops.c > > +++ b/fs/xfs/xfs_fsops.c > > @@ -128,6 +128,14 @@ xfs_growfs_data_private( > > nb - mp->m_sb.sb_dblocks); > > if (id.nfree) > > xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree); > > + > > + /* > > + * update in-core counters now to reflect the real numbers > > + * (especially sb_fdblocks) > > + */ > > Could you update the comment to explain why we do this? For example: > > "Sync sb counters now to reflect the updated values. This is > particularly important for shrink because the write verifier will fail > if sb_fdblocks is ever larger than sb_dblocks." > Thanks for the review/suggestion! I updated the comment in "[PATCH v6 6/7] xfs: support shrinking unused space in the last AG", since shrinking functionality is somewhat landed after [PATCH 6/7]... If that looks worse than changing directly here, I will shift/update the comment in the next version. Thanks, Gao Xiang > Brian >
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index a2a407039227..2e490fb75832 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -128,6 +128,14 @@ xfs_growfs_data_private( nb - mp->m_sb.sb_dblocks); if (id.nfree) xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree); + + /* + * update in-core counters now to reflect the real numbers + * (especially sb_fdblocks) + */ + if (xfs_sb_version_haslazysbcount(&mp->m_sb)) + xfs_log_sb(tp); + xfs_trans_set_sync(tp); error = xfs_trans_commit(tp); if (error)
sb_fdblocks will be updated lazily if lazysbcount is enabled, therefore when shrinking the filesystem sb_fdblocks could be larger than sb_dblocks and xfs_validate_sb_write() would fail. Even for growfs case, it'd be better to update lazy sb counters immediately to reflect the real sb counters. Signed-off-by: Gao Xiang <hsiangkao@redhat.com> --- fs/xfs/xfs_fsops.c | 8 ++++++++ 1 file changed, 8 insertions(+)