* [PATCHSET 0/5] xfs: other stuff for 5.15
@ 2021-08-05 7:00 Darrick J. Wong
2021-08-05 7:00 ` [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs Darrick J. Wong
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Darrick J. Wong @ 2021-08-05 7:00 UTC (permalink / raw)
To: djwong; +Cc: Bill O'Donnell, Carlos Maiolino, linux-xfs
Hi all,
This is the usual grab bag of miscellaneous things for 5.15: continued
cleanups of the perag iteration code, dropping EXPERIMENTAL warnings,
making log recovery fail louder, dorky whitespace fixes, etc.
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This is an extraordinary way to destroy everything. Enjoy!
Comments and questions are, as always, welcome.
--D
kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=other-stuff-for-5.15
---
fs/xfs/Makefile | 3 ++
fs/xfs/libxfs/xfs_ag.h | 66 +++++++++++++++++++++++++---------------
fs/xfs/libxfs/xfs_attr_leaf.c | 2 +
fs/xfs/libxfs/xfs_format.h | 2 +
fs/xfs/libxfs/xfs_ialloc.c | 2 +
fs/xfs/libxfs/xfs_rmap_btree.h | 2 +
fs/xfs/libxfs/xfs_sb.c | 17 +++++-----
fs/xfs/libxfs/xfs_types.c | 6 +---
fs/xfs/scrub/agheader.c | 23 ++++++++++----
fs/xfs/scrub/agheader_repair.c | 3 --
fs/xfs/scrub/bmap.c | 10 ++----
fs/xfs/scrub/btree.c | 2 +
fs/xfs/scrub/common.c | 48 ++++++++++++-----------------
fs/xfs/scrub/common.h | 18 ++++++++++-
fs/xfs/scrub/fscounters.c | 43 ++++++++++++--------------
fs/xfs/scrub/inode.c | 2 +
fs/xfs/xfs_bmap_item.c | 3 ++
fs/xfs/xfs_extent_busy.c | 11 +++----
fs/xfs/xfs_extfree_item.c | 3 ++
fs/xfs/xfs_fsmap.c | 25 +++++----------
fs/xfs/xfs_fsops.c | 12 ++-----
fs/xfs/xfs_health.c | 9 ++---
fs/xfs/xfs_icache.c | 27 +++++-----------
fs/xfs/xfs_iwalk.c | 41 ++++++++++---------------
fs/xfs/xfs_log_recover.c | 21 +++++--------
fs/xfs/xfs_refcount_item.c | 3 ++
fs/xfs/xfs_reflink.c | 10 ++----
fs/xfs/xfs_rmap_item.c | 3 ++
fs/xfs/xfs_super.c | 8 -----
29 files changed, 207 insertions(+), 218 deletions(-)
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs 2021-08-05 7:00 [PATCHSET 0/5] xfs: other stuff for 5.15 Darrick J. Wong @ 2021-08-05 7:00 ` Darrick J. Wong 2021-08-06 5:45 ` Chandan Babu R 2021-08-09 14:51 ` Christoph Hellwig 2021-08-05 7:00 ` [PATCH 2/5] xfs: drop experimental warnings for bigtime and inobtcount Darrick J. Wong ` (3 subsequent siblings) 4 siblings, 2 replies; 15+ messages in thread From: Darrick J. Wong @ 2021-08-05 7:00 UTC (permalink / raw) To: djwong; +Cc: linux-xfs From: Darrick J. Wong <djwong@kernel.org> Fix a few whitespace errors such as spaces at the end of the line, etc. This gets us back to something more closely resembling parity. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- fs/xfs/libxfs/xfs_attr_leaf.c | 2 +- fs/xfs/libxfs/xfs_format.h | 2 +- fs/xfs/libxfs/xfs_ialloc.c | 2 +- fs/xfs/libxfs/xfs_rmap_btree.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index b910bd209949..b277e0511cdd 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -576,7 +576,7 @@ xfs_attr_shortform_bytesfit( switch (dp->i_df.if_format) { case XFS_DINODE_FMT_EXTENTS: /* - * If there is no attr fork and the data fork is extents, + * If there is no attr fork and the data fork is extents, * determine if creating the default attr fork will result * in the extents form migrating to btree. If so, the * minimum offset only needs to be the space required for diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h index 76e2461b9e66..37570cf0537e 100644 --- a/fs/xfs/libxfs/xfs_format.h +++ b/fs/xfs/libxfs/xfs_format.h @@ -9,7 +9,7 @@ /* * XFS On Disk Format Definitions * - * This header file defines all the on-disk format definitions for + * This header file defines all the on-disk format definitions for * general XFS objects. Directory and attribute related objects are defined in * xfs_da_format.h, which log and log item formats are defined in * xfs_log_format.h. Everything else goes here. diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c index aaf8805a82df..19eb7ec0103f 100644 --- a/fs/xfs/libxfs/xfs_ialloc.c +++ b/fs/xfs/libxfs/xfs_ialloc.c @@ -1994,7 +1994,7 @@ xfs_difree_inobt( goto error0; } - /* + /* * Change the inode free counts and log the ag/sb changes. */ be32_add_cpu(&agi->agi_freecount, 1); diff --git a/fs/xfs/libxfs/xfs_rmap_btree.h b/fs/xfs/libxfs/xfs_rmap_btree.h index 88d8d18788a2..f2eee6572af4 100644 --- a/fs/xfs/libxfs/xfs_rmap_btree.h +++ b/fs/xfs/libxfs/xfs_rmap_btree.h @@ -59,4 +59,4 @@ extern xfs_extlen_t xfs_rmapbt_max_size(struct xfs_mount *mp, extern int xfs_rmapbt_calc_reserves(struct xfs_mount *mp, struct xfs_trans *tp, struct xfs_perag *pag, xfs_extlen_t *ask, xfs_extlen_t *used); -#endif /* __XFS_RMAP_BTREE_H__ */ +#endif /* __XFS_RMAP_BTREE_H__ */ ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs 2021-08-05 7:00 ` [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs Darrick J. Wong @ 2021-08-06 5:45 ` Chandan Babu R 2021-08-09 14:51 ` Christoph Hellwig 1 sibling, 0 replies; 15+ messages in thread From: Chandan Babu R @ 2021-08-06 5:45 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On 05 Aug 2021 at 00:00, "Darrick J. Wong" <djwong@kernel.org> wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Fix a few whitespace errors such as spaces at the end of the line, etc. > This gets us back to something more closely resembling parity. > Looks good to me. Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/libxfs/xfs_attr_leaf.c | 2 +- > fs/xfs/libxfs/xfs_format.h | 2 +- > fs/xfs/libxfs/xfs_ialloc.c | 2 +- > fs/xfs/libxfs/xfs_rmap_btree.h | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c > index b910bd209949..b277e0511cdd 100644 > --- a/fs/xfs/libxfs/xfs_attr_leaf.c > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c > @@ -576,7 +576,7 @@ xfs_attr_shortform_bytesfit( > switch (dp->i_df.if_format) { > case XFS_DINODE_FMT_EXTENTS: > /* > - * If there is no attr fork and the data fork is extents, > + * If there is no attr fork and the data fork is extents, > * determine if creating the default attr fork will result > * in the extents form migrating to btree. If so, the > * minimum offset only needs to be the space required for > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h > index 76e2461b9e66..37570cf0537e 100644 > --- a/fs/xfs/libxfs/xfs_format.h > +++ b/fs/xfs/libxfs/xfs_format.h > @@ -9,7 +9,7 @@ > /* > * XFS On Disk Format Definitions > * > - * This header file defines all the on-disk format definitions for > + * This header file defines all the on-disk format definitions for > * general XFS objects. Directory and attribute related objects are defined in > * xfs_da_format.h, which log and log item formats are defined in > * xfs_log_format.h. Everything else goes here. > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c > index aaf8805a82df..19eb7ec0103f 100644 > --- a/fs/xfs/libxfs/xfs_ialloc.c > +++ b/fs/xfs/libxfs/xfs_ialloc.c > @@ -1994,7 +1994,7 @@ xfs_difree_inobt( > goto error0; > } > > - /* > + /* > * Change the inode free counts and log the ag/sb changes. > */ > be32_add_cpu(&agi->agi_freecount, 1); > diff --git a/fs/xfs/libxfs/xfs_rmap_btree.h b/fs/xfs/libxfs/xfs_rmap_btree.h > index 88d8d18788a2..f2eee6572af4 100644 > --- a/fs/xfs/libxfs/xfs_rmap_btree.h > +++ b/fs/xfs/libxfs/xfs_rmap_btree.h > @@ -59,4 +59,4 @@ extern xfs_extlen_t xfs_rmapbt_max_size(struct xfs_mount *mp, > extern int xfs_rmapbt_calc_reserves(struct xfs_mount *mp, struct xfs_trans *tp, > struct xfs_perag *pag, xfs_extlen_t *ask, xfs_extlen_t *used); > > -#endif /* __XFS_RMAP_BTREE_H__ */ > +#endif /* __XFS_RMAP_BTREE_H__ */ -- chandan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs 2021-08-05 7:00 ` [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs Darrick J. Wong 2021-08-06 5:45 ` Chandan Babu R @ 2021-08-09 14:51 ` Christoph Hellwig 1 sibling, 0 replies; 15+ messages in thread From: Christoph Hellwig @ 2021-08-09 14:51 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/5] xfs: drop experimental warnings for bigtime and inobtcount 2021-08-05 7:00 [PATCHSET 0/5] xfs: other stuff for 5.15 Darrick J. Wong 2021-08-05 7:00 ` [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs Darrick J. Wong @ 2021-08-05 7:00 ` Darrick J. Wong 2021-08-06 5:51 ` Chandan Babu R 2021-08-09 14:53 ` Christoph Hellwig 2021-08-05 7:01 ` [PATCH 3/5] xfs: automatic resource cleanup of for_each_perag* Darrick J. Wong ` (2 subsequent siblings) 4 siblings, 2 replies; 15+ messages in thread From: Darrick J. Wong @ 2021-08-05 7:00 UTC (permalink / raw) To: djwong; +Cc: Carlos Maiolino, Bill O'Donnell, linux-xfs From: Darrick J. Wong <djwong@kernel.org> These two features were merged a year ago, userspace tooling have been merged, and no serious errors have been reported by the developers. Drop the experimental tag to encourage wider testing. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by: Bill O'Donnell <billodo@redhat.com> --- fs/xfs/xfs_super.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index 2bab18ed73b9..c4ba5c712284 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -1599,10 +1599,6 @@ xfs_fs_fill_super( if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5) sb->s_flags |= SB_I_VERSION; - if (xfs_sb_version_hasbigtime(&mp->m_sb)) - xfs_warn(mp, - "EXPERIMENTAL big timestamp feature in use. Use at your own risk!"); - if (mp->m_flags & XFS_MOUNT_DAX_ALWAYS) { bool rtdev_is_dax = false, datadev_is_dax; @@ -1658,10 +1654,6 @@ xfs_fs_fill_super( goto out_filestream_unmount; } - if (xfs_sb_version_hasinobtcounts(&mp->m_sb)) - xfs_warn(mp, - "EXPERIMENTAL inode btree counters feature in use. Use at your own risk!"); - error = xfs_mountfs(mp); if (error) goto out_filestream_unmount; ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] xfs: drop experimental warnings for bigtime and inobtcount 2021-08-05 7:00 ` [PATCH 2/5] xfs: drop experimental warnings for bigtime and inobtcount Darrick J. Wong @ 2021-08-06 5:51 ` Chandan Babu R 2021-08-09 14:53 ` Christoph Hellwig 1 sibling, 0 replies; 15+ messages in thread From: Chandan Babu R @ 2021-08-06 5:51 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Carlos Maiolino, Bill O'Donnell, linux-xfs On 05 Aug 2021 at 00:00, "Darrick J. Wong" <djwong@kernel.org> wrote: > From: Darrick J. Wong <djwong@kernel.org> > > These two features were merged a year ago, userspace tooling have been > merged, and no serious errors have been reported by the developers. > Drop the experimental tag to encourage wider testing. > Looks good to me. Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> > Reviewed-by: Bill O'Donnell <billodo@redhat.com> > --- > fs/xfs/xfs_super.c | 8 -------- > 1 file changed, 8 deletions(-) > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index 2bab18ed73b9..c4ba5c712284 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -1599,10 +1599,6 @@ xfs_fs_fill_super( > if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5) > sb->s_flags |= SB_I_VERSION; > > - if (xfs_sb_version_hasbigtime(&mp->m_sb)) > - xfs_warn(mp, > - "EXPERIMENTAL big timestamp feature in use. Use at your own risk!"); > - > if (mp->m_flags & XFS_MOUNT_DAX_ALWAYS) { > bool rtdev_is_dax = false, datadev_is_dax; > > @@ -1658,10 +1654,6 @@ xfs_fs_fill_super( > goto out_filestream_unmount; > } > > - if (xfs_sb_version_hasinobtcounts(&mp->m_sb)) > - xfs_warn(mp, > - "EXPERIMENTAL inode btree counters feature in use. Use at your own risk!"); > - > error = xfs_mountfs(mp); > if (error) > goto out_filestream_unmount; -- chandan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] xfs: drop experimental warnings for bigtime and inobtcount 2021-08-05 7:00 ` [PATCH 2/5] xfs: drop experimental warnings for bigtime and inobtcount Darrick J. Wong 2021-08-06 5:51 ` Chandan Babu R @ 2021-08-09 14:53 ` Christoph Hellwig 1 sibling, 0 replies; 15+ messages in thread From: Christoph Hellwig @ 2021-08-09 14:53 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Carlos Maiolino, Bill O'Donnell, linux-xfs Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/5] xfs: automatic resource cleanup of for_each_perag* 2021-08-05 7:00 [PATCHSET 0/5] xfs: other stuff for 5.15 Darrick J. Wong 2021-08-05 7:00 ` [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs Darrick J. Wong 2021-08-05 7:00 ` [PATCH 2/5] xfs: drop experimental warnings for bigtime and inobtcount Darrick J. Wong @ 2021-08-05 7:01 ` Darrick J. Wong 2021-08-06 7:15 ` Chandan Babu R 2021-08-09 15:07 ` Christoph Hellwig 2021-08-05 7:01 ` [PATCH 4/5] xfs: grab active perag ref when reading AG headers Darrick J. Wong 2021-08-05 7:01 ` [PATCH 5/5] xfs: dump log intent items that cannot be recovered due to corruption Darrick J. Wong 4 siblings, 2 replies; 15+ messages in thread From: Darrick J. Wong @ 2021-08-05 7:01 UTC (permalink / raw) To: djwong; +Cc: linux-xfs From: Darrick J. Wong <djwong@kernel.org> Jan Kara pointed me to a magic gcc attribute that schedules a finalizer function to run against any automatic variable that's going out of scope. Provided nobody minds compiling XFS with gnu99 instead of c89, we can use this capability to define a loop-scope iterator variable that will trigger the automatic finalizer, which means that we don't have to burden for_each_perag* callers with the necessity of manually putting the perag structures when exiting the loop body. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- fs/xfs/Makefile | 3 ++ fs/xfs/libxfs/xfs_ag.h | 66 +++++++++++++++++++++++++++++---------------- fs/xfs/libxfs/xfs_sb.c | 17 ++++++------ fs/xfs/libxfs/xfs_types.c | 6 +--- fs/xfs/scrub/bmap.c | 8 +---- fs/xfs/scrub/fscounters.c | 41 ++++++++++++---------------- fs/xfs/xfs_extent_busy.c | 11 +++----- fs/xfs/xfs_fsmap.c | 25 ++++++----------- fs/xfs/xfs_fsops.c | 12 +++----- fs/xfs/xfs_health.c | 9 +++--- fs/xfs/xfs_icache.c | 27 ++++++------------ fs/xfs/xfs_iwalk.c | 41 +++++++++++----------------- fs/xfs/xfs_log_recover.c | 21 ++++++-------- fs/xfs/xfs_reflink.c | 10 ++----- 14 files changed, 135 insertions(+), 162 deletions(-) diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile index 04611a1068b4..e482cb43743d 100644 --- a/fs/xfs/Makefile +++ b/fs/xfs/Makefile @@ -7,6 +7,9 @@ ccflags-y += -I $(srctree)/$(src) # needed for trace events ccflags-y += -I $(srctree)/$(src)/libxfs +# Required for for_each_perag* +ccflags-y += -std=gnu99 + obj-$(CONFIG_XFS_FS) += xfs.o # this one should be compiled first, as the tracing macros can easily blow up diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h index 4c6f9045baca..5141e8936912 100644 --- a/fs/xfs/libxfs/xfs_ag.h +++ b/fs/xfs/libxfs/xfs_ag.h @@ -116,35 +116,53 @@ void xfs_perag_put(struct xfs_perag *pag); /* * Perag iteration APIs - * - * XXX: for_each_perag_range() usage really needs an iterator to clean up when - * we terminate at end_agno because we may have taken a reference to the perag - * beyond end_agno. Right now callers have to be careful to catch and clean that - * up themselves. This is not necessary for the callers of for_each_perag() and - * for_each_perag_from() because they terminate at sb_agcount where there are - * no perag structures in tree beyond end_agno. */ -#define for_each_perag_range(mp, next_agno, end_agno, pag) \ - for ((pag) = xfs_perag_get((mp), (next_agno)); \ - (pag) != NULL && (next_agno) <= (end_agno); \ - (next_agno) = (pag)->pag_agno + 1, \ - xfs_perag_put(pag), \ - (pag) = xfs_perag_get((mp), (next_agno))) +struct xfs_perag_iter { + struct xfs_perag *pag; + xfs_agnumber_t __agno; +}; -#define for_each_perag_from(mp, next_agno, pag) \ - for_each_perag_range((mp), (next_agno), (mp)->m_sb.sb_agcount, (pag)) +static inline void +xfs_perag_iter_cleanup(struct xfs_perag_iter *pagit) +{ + if (pagit->pag) + xfs_perag_put(pagit->pag); + pagit->pag = NULL; + pagit->__agno = NULLAGNUMBER; +} +#define XFS_PERAG_ITER_INIT(mp, name, first_agno) \ + struct xfs_perag_iter name \ + __attribute__((__cleanup__(xfs_perag_iter_cleanup))) = \ + { .__agno = (first_agno), \ + .pag = xfs_perag_get((mp), (first_agno)), } -#define for_each_perag(mp, agno, pag) \ - (agno) = 0; \ - for_each_perag_from((mp), (agno), (pag)) +#define XFS_PERAG_TAG_ITER_INIT(mp, name, first_agno, tag) \ + struct xfs_perag_iter name \ + __attribute__((__cleanup__(xfs_perag_iter_cleanup))) = \ + { .__agno = (first_agno), \ + .pag = xfs_perag_get_tag((mp), (first_agno), (tag)), } -#define for_each_perag_tag(mp, agno, pag, tag) \ - for ((agno) = 0, (pag) = xfs_perag_get_tag((mp), 0, (tag)); \ - (pag) != NULL; \ - (agno) = (pag)->pag_agno + 1, \ - xfs_perag_put(pag), \ - (pag) = xfs_perag_get_tag((mp), (agno), (tag))) +/* Perag iteration APIs */ +#define for_each_perag_range(mp, name, start_agno, end_agno) \ + for (XFS_PERAG_ITER_INIT((mp), (name), (start_agno)); \ + (name).pag != NULL && (name).__agno <= (end_agno); \ + (name).__agno = (name).pag->pag_agno + 1, \ + xfs_perag_put((name).pag), \ + (name).pag = xfs_perag_get((mp), (name).__agno)) + +#define for_each_perag_from(mp, name, start_agno) \ + for_each_perag_range((mp), (name), (start_agno), (mp)->m_sb.sb_agcount) + +#define for_each_perag(mp, name) \ + for_each_perag_from((mp), (name), 0) + +#define for_each_perag_tag(mp, name, tag) \ + for (XFS_PERAG_TAG_ITER_INIT((mp), (name), 0, (tag)); \ + (name).pag != NULL; \ + (name).__agno = (name).pag->pag_agno + 1, \ + xfs_perag_put((name).pag), \ + (name).pag = xfs_perag_get_tag((mp), (name).__agno, (tag))) struct aghdr_init_data { /* per ag data */ diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index 04f5386446db..a0a07ebaf41b 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c @@ -856,18 +856,19 @@ int xfs_update_secondary_sbs( struct xfs_mount *mp) { - struct xfs_perag *pag; - xfs_agnumber_t agno = 1; + xfs_agnumber_t last_agno = 0; int saved_error = 0; int error = 0; LIST_HEAD (buffer_list); /* update secondary superblocks. */ - for_each_perag_from(mp, agno, pag) { + for_each_perag_from(mp, iter, 1) { struct xfs_buf *bp; + last_agno = iter.pag->pag_agno; + error = xfs_buf_get(mp->m_ddev_targp, - XFS_AG_DADDR(mp, pag->pag_agno, XFS_SB_DADDR), + XFS_AG_DADDR(mp, last_agno, XFS_SB_DADDR), XFS_FSS_TO_BB(mp, 1), &bp); /* * If we get an error reading or writing alternate superblocks, @@ -879,7 +880,7 @@ xfs_update_secondary_sbs( if (error) { xfs_warn(mp, "error allocating secondary superblock for ag %d", - pag->pag_agno); + last_agno); if (!saved_error) saved_error = error; continue; @@ -893,14 +894,14 @@ xfs_update_secondary_sbs( xfs_buf_relse(bp); /* don't hold too many buffers at once */ - if (agno % 16) + if (last_agno % 16) continue; error = xfs_buf_delwri_submit(&buffer_list); if (error) { xfs_warn(mp, "write error %d updating a secondary superblock near ag %d", - error, pag->pag_agno); + error, last_agno); if (!saved_error) saved_error = error; continue; @@ -910,7 +911,7 @@ xfs_update_secondary_sbs( if (error) { xfs_warn(mp, "write error %d updating a secondary superblock near ag %d", - error, agno); + error, last_agno); } return saved_error ? saved_error : error; diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c index e8f4abee7892..8654b46b7c60 100644 --- a/fs/xfs/libxfs/xfs_types.c +++ b/fs/xfs/libxfs/xfs_types.c @@ -223,16 +223,14 @@ xfs_icount_range( unsigned long long *max) { unsigned long long nr_inos = 0; - struct xfs_perag *pag; - xfs_agnumber_t agno; /* root, rtbitmap, rtsum all live in the first chunk */ *min = XFS_INODES_PER_CHUNK; - for_each_perag(mp, agno, pag) { + for_each_perag(mp, iter) { xfs_agino_t first, last; - xfs_agino_range(mp, agno, &first, &last); + xfs_agino_range(mp, iter.pag->pag_agno, &first, &last); nr_inos += last - first + 1; } *max = nr_inos; diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c index 1d146c9d9de1..14e952d116f4 100644 --- a/fs/xfs/scrub/bmap.c +++ b/fs/xfs/scrub/bmap.c @@ -576,8 +576,6 @@ xchk_bmap_check_rmaps( int whichfork) { struct xfs_ifork *ifp = XFS_IFORK_PTR(sc->ip, whichfork); - struct xfs_perag *pag; - xfs_agnumber_t agno; bool zero_size; int error; @@ -609,15 +607,13 @@ xchk_bmap_check_rmaps( (zero_size || ifp->if_nextents > 0)) return 0; - for_each_perag(sc->mp, agno, pag) { - error = xchk_bmap_check_ag_rmaps(sc, whichfork, pag); + for_each_perag(sc->mp, iter) { + error = xchk_bmap_check_ag_rmaps(sc, whichfork, iter.pag); if (error) break; if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) break; } - if (pag) - xfs_perag_put(pag); return error; } diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c index fd7941e04ae1..e03577af63d9 100644 --- a/fs/xfs/scrub/fscounters.c +++ b/fs/xfs/scrub/fscounters.c @@ -67,21 +67,21 @@ xchk_fscount_warmup( struct xfs_mount *mp = sc->mp; struct xfs_buf *agi_bp = NULL; struct xfs_buf *agf_bp = NULL; - struct xfs_perag *pag = NULL; - xfs_agnumber_t agno; int error = 0; - for_each_perag(mp, agno, pag) { + for_each_perag(mp, iter) { if (xchk_should_terminate(sc, &error)) break; - if (pag->pagi_init && pag->pagf_init) + if (iter.pag->pagi_init && iter.pag->pagf_init) continue; /* Lock both AG headers. */ - error = xfs_ialloc_read_agi(mp, sc->tp, agno, &agi_bp); + error = xfs_ialloc_read_agi(mp, sc->tp, iter.pag->pag_agno, + &agi_bp); if (error) break; - error = xfs_alloc_read_agf(mp, sc->tp, agno, 0, &agf_bp); + error = xfs_alloc_read_agf(mp, sc->tp, iter.pag->pag_agno, 0, + &agf_bp); if (error) break; @@ -89,7 +89,7 @@ xchk_fscount_warmup( * These are supposed to be initialized by the header read * function. */ - if (!pag->pagi_init || !pag->pagf_init) { + if (!iter.pag->pagi_init || !iter.pag->pagf_init) { error = -EFSCORRUPTED; break; } @@ -104,8 +104,6 @@ xchk_fscount_warmup( xfs_buf_relse(agf_bp); if (agi_bp) xfs_buf_relse(agi_bp); - if (pag) - xfs_perag_put(pag); return error; } @@ -179,9 +177,7 @@ xchk_fscount_aggregate_agcounts( struct xchk_fscounters *fsc) { struct xfs_mount *mp = sc->mp; - struct xfs_perag *pag; uint64_t delayed; - xfs_agnumber_t agno; int tries = 8; int error = 0; @@ -190,27 +186,28 @@ xchk_fscount_aggregate_agcounts( fsc->ifree = 0; fsc->fdblocks = 0; - for_each_perag(mp, agno, pag) { + for_each_perag(mp, iter) { if (xchk_should_terminate(sc, &error)) break; /* This somehow got unset since the warmup? */ - if (!pag->pagi_init || !pag->pagf_init) { + if (!iter.pag->pagi_init || !iter.pag->pagf_init) { error = -EFSCORRUPTED; break; } /* Count all the inodes */ - fsc->icount += pag->pagi_count; - fsc->ifree += pag->pagi_freecount; + fsc->icount += iter.pag->pagi_count; + fsc->ifree += iter.pag->pagi_freecount; /* Add up the free/freelist/bnobt/cntbt blocks */ - fsc->fdblocks += pag->pagf_freeblks; - fsc->fdblocks += pag->pagf_flcount; + fsc->fdblocks += iter.pag->pagf_freeblks; + fsc->fdblocks += iter.pag->pagf_flcount; if (xfs_sb_version_haslazysbcount(&sc->mp->m_sb)) { - fsc->fdblocks += pag->pagf_btreeblks; + fsc->fdblocks += iter.pag->pagf_btreeblks; } else { - error = xchk_fscount_btreeblks(sc, fsc, agno); + error = xchk_fscount_btreeblks(sc, fsc, + iter.pag->pag_agno); if (error) break; } @@ -219,12 +216,10 @@ xchk_fscount_aggregate_agcounts( * Per-AG reservations are taken out of the incore counters, * so they must be left out of the free blocks computation. */ - fsc->fdblocks -= pag->pag_meta_resv.ar_reserved; - fsc->fdblocks -= pag->pag_rmapbt_resv.ar_orig_reserved; + fsc->fdblocks -= iter.pag->pag_meta_resv.ar_reserved; + fsc->fdblocks -= iter.pag->pag_rmapbt_resv.ar_orig_reserved; } - if (pag) - xfs_perag_put(pag); if (error) return error; diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c index ad22a003f959..fe85df915707 100644 --- a/fs/xfs/xfs_extent_busy.c +++ b/fs/xfs/xfs_extent_busy.c @@ -593,18 +593,17 @@ void xfs_extent_busy_wait_all( struct xfs_mount *mp) { - struct xfs_perag *pag; DEFINE_WAIT (wait); - xfs_agnumber_t agno; - for_each_perag(mp, agno, pag) { + for_each_perag(mp, iter) { do { - prepare_to_wait(&pag->pagb_wait, &wait, TASK_KILLABLE); - if (RB_EMPTY_ROOT(&pag->pagb_tree)) + prepare_to_wait(&iter.pag->pagb_wait, &wait, + TASK_KILLABLE); + if (RB_EMPTY_ROOT(&iter.pag->pagb_tree)) break; schedule(); } while (1); - finish_wait(&pag->pagb_wait, &wait); + finish_wait(&iter.pag->pagb_wait, &wait); } } diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c index 7d0b09c1366e..cdf44db29973 100644 --- a/fs/xfs/xfs_fsmap.c +++ b/fs/xfs/xfs_fsmap.c @@ -573,7 +573,6 @@ __xfs_getfsmap_datadev( void *priv) { struct xfs_mount *mp = tp->t_mountp; - struct xfs_perag *pag; struct xfs_btree_cur *bt_cur = NULL; xfs_fsblock_t start_fsb; xfs_fsblock_t end_fsb; @@ -612,13 +611,13 @@ __xfs_getfsmap_datadev( start_ag = XFS_FSB_TO_AGNO(mp, start_fsb); end_ag = XFS_FSB_TO_AGNO(mp, end_fsb); - for_each_perag_range(mp, start_ag, end_ag, pag) { + for_each_perag_range(mp, iter, start_ag, end_ag) { /* * Set the AG high key from the fsmap high key if this * is the last AG that we're querying. */ - info->pag = pag; - if (pag->pag_agno == end_ag) { + info->pag = iter.pag; + if (iter.pag->pag_agno == end_ag) { info->high.rm_startblock = XFS_FSB_TO_AGBNO(mp, end_fsb); info->high.rm_offset = XFS_BB_TO_FSBT(mp, @@ -636,14 +635,14 @@ __xfs_getfsmap_datadev( info->agf_bp = NULL; } - error = xfs_alloc_read_agf(mp, tp, pag->pag_agno, 0, + error = xfs_alloc_read_agf(mp, tp, iter.pag->pag_agno, 0, &info->agf_bp); if (error) break; - trace_xfs_fsmap_low_key(mp, info->dev, pag->pag_agno, + trace_xfs_fsmap_low_key(mp, info->dev, iter.pag->pag_agno, &info->low); - trace_xfs_fsmap_high_key(mp, info->dev, pag->pag_agno, + trace_xfs_fsmap_high_key(mp, info->dev, iter.pag->pag_agno, &info->high); error = query_fn(tp, info, &bt_cur, priv); @@ -654,7 +653,7 @@ __xfs_getfsmap_datadev( * Set the AG low key to the start of the AG prior to * moving on to the next AG. */ - if (pag->pag_agno == start_ag) { + if (iter.pag->pag_agno == start_ag) { info->low.rm_startblock = 0; info->low.rm_owner = 0; info->low.rm_offset = 0; @@ -666,7 +665,7 @@ __xfs_getfsmap_datadev( * before we drop the reference to the perag when the loop * terminates. */ - if (pag->pag_agno == end_ag) { + if (iter.pag->pag_agno == end_ag) { info->last = true; error = query_fn(tp, info, &bt_cur, priv); if (error) @@ -682,13 +681,7 @@ __xfs_getfsmap_datadev( xfs_trans_brelse(tp, info->agf_bp); info->agf_bp = NULL; } - if (info->pag) { - xfs_perag_put(info->pag); - info->pag = NULL; - } else if (pag) { - /* loop termination case */ - xfs_perag_put(pag); - } + info->pag = NULL; return error; } diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index 6ed29b158312..31f585a525e8 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -570,14 +570,12 @@ int xfs_fs_reserve_ag_blocks( struct xfs_mount *mp) { - xfs_agnumber_t agno; - struct xfs_perag *pag; int error = 0; int err2; mp->m_finobt_nores = false; - for_each_perag(mp, agno, pag) { - err2 = xfs_ag_resv_init(pag, NULL); + for_each_perag(mp, iter) { + err2 = xfs_ag_resv_init(iter.pag, NULL); if (err2 && !error) error = err2; } @@ -598,13 +596,11 @@ int xfs_fs_unreserve_ag_blocks( struct xfs_mount *mp) { - xfs_agnumber_t agno; - struct xfs_perag *pag; int error = 0; int err2; - for_each_perag(mp, agno, pag) { - err2 = xfs_ag_resv_free(pag); + for_each_perag(mp, iter) { + err2 = xfs_ag_resv_free(iter.pag); if (err2 && !error) error = err2; } diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c index eb10eacabc8f..715a987b8204 100644 --- a/fs/xfs/xfs_health.c +++ b/fs/xfs/xfs_health.c @@ -24,8 +24,6 @@ void xfs_health_unmount( struct xfs_mount *mp) { - struct xfs_perag *pag; - xfs_agnumber_t agno; unsigned int sick = 0; unsigned int checked = 0; bool warn = false; @@ -34,10 +32,11 @@ xfs_health_unmount( return; /* Measure AG corruption levels. */ - for_each_perag(mp, agno, pag) { - xfs_ag_measure_sickness(pag, &sick, &checked); + for_each_perag(mp, iter) { + xfs_ag_measure_sickness(iter.pag, &sick, &checked); if (sick) { - trace_xfs_ag_unfixed_corruption(mp, agno, sick); + trace_xfs_ag_unfixed_corruption(mp, iter.pag->pag_agno, + sick); warn = true; } } diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c index 6741e27603ad..1d4006df4066 100644 --- a/fs/xfs/xfs_icache.c +++ b/fs/xfs/xfs_icache.c @@ -1368,14 +1368,11 @@ void xfs_blockgc_stop( struct xfs_mount *mp) { - struct xfs_perag *pag; - xfs_agnumber_t agno; - if (!xfs_clear_blockgc_enabled(mp)) return; - for_each_perag(mp, agno, pag) - cancel_delayed_work_sync(&pag->pag_blockgc_work); + for_each_perag(mp, iter) + cancel_delayed_work_sync(&iter.pag->pag_blockgc_work); trace_xfs_blockgc_stop(mp, __return_address); } @@ -1384,15 +1381,12 @@ void xfs_blockgc_start( struct xfs_mount *mp) { - struct xfs_perag *pag; - xfs_agnumber_t agno; - if (xfs_set_blockgc_enabled(mp)) return; trace_xfs_blockgc_start(mp, __return_address); - for_each_perag_tag(mp, agno, pag, XFS_ICI_BLOCKGC_TAG) - xfs_blockgc_queue(pag); + for_each_perag_tag(mp, iter, XFS_ICI_BLOCKGC_TAG) + xfs_blockgc_queue(iter.pag); } /* Don't try to run block gc on an inode that's in any of these states. */ @@ -1515,9 +1509,6 @@ void xfs_blockgc_flush_all( struct xfs_mount *mp) { - struct xfs_perag *pag; - xfs_agnumber_t agno; - trace_xfs_blockgc_flush_all(mp, __return_address); /* @@ -1525,12 +1516,12 @@ xfs_blockgc_flush_all( * wasn't queued, it will not be requeued. Then flush whatever's * left. */ - for_each_perag_tag(mp, agno, pag, XFS_ICI_BLOCKGC_TAG) - mod_delayed_work(pag->pag_mount->m_blockgc_wq, - &pag->pag_blockgc_work, 0); + for_each_perag_tag(mp, iter, XFS_ICI_BLOCKGC_TAG) + mod_delayed_work(mp->m_blockgc_wq, &iter.pag->pag_blockgc_work, + 0); - for_each_perag_tag(mp, agno, pag, XFS_ICI_BLOCKGC_TAG) - flush_delayed_work(&pag->pag_blockgc_work); + for_each_perag_tag(mp, iter, XFS_ICI_BLOCKGC_TAG) + flush_delayed_work(&iter.pag->pag_blockgc_work); xfs_inodegc_flush(mp); } diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c index 7558486f4937..0c06a59aec79 100644 --- a/fs/xfs/xfs_iwalk.c +++ b/fs/xfs/xfs_iwalk.c @@ -568,30 +568,27 @@ xfs_iwalk( .pwork = XFS_PWORK_SINGLE_THREADED, .lastino = NULLFSINO, }; - struct xfs_perag *pag; - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, startino); + xfs_agnumber_t start_agno = XFS_INO_TO_AGNO(mp, startino); int error; - ASSERT(agno < mp->m_sb.sb_agcount); + ASSERT(start_agno < mp->m_sb.sb_agcount); ASSERT(!(flags & ~XFS_IWALK_FLAGS_ALL)); error = xfs_iwalk_alloc(&iwag); if (error) return error; - for_each_perag_from(mp, agno, pag) { - iwag.pag = pag; + for_each_perag_from(mp, iter, start_agno) { + iwag.pag = iter.pag; error = xfs_iwalk_ag(&iwag); if (error) break; - iwag.startino = XFS_AGINO_TO_INO(mp, agno + 1, 0); + iwag.startino = XFS_AGINO_TO_INO(mp, iter.pag->pag_agno + 1, 0); if (flags & XFS_INOBT_WALK_SAME_AG) break; iwag.pag = NULL; } - if (iwag.pag) - xfs_perag_put(pag); xfs_iwalk_free(&iwag); return error; } @@ -646,18 +643,17 @@ xfs_iwalk_threaded( void *data) { struct xfs_pwork_ctl pctl; - struct xfs_perag *pag; - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, startino); + xfs_agnumber_t start_agno = XFS_INO_TO_AGNO(mp, startino); int error; - ASSERT(agno < mp->m_sb.sb_agcount); + ASSERT(start_agno < mp->m_sb.sb_agcount); ASSERT(!(flags & ~XFS_IWALK_FLAGS_ALL)); error = xfs_pwork_init(mp, &pctl, xfs_iwalk_ag_work, "xfs_iwalk"); if (error) return error; - for_each_perag_from(mp, agno, pag) { + for_each_perag_from(mp, iter, start_agno) { struct xfs_iwalk_ag *iwag; if (xfs_pwork_ctl_want_abort(&pctl)) @@ -670,20 +666,18 @@ xfs_iwalk_threaded( * perag is being handed off to async work, so take another * reference for the async work to release. */ - atomic_inc(&pag->pag_ref); - iwag->pag = pag; + atomic_inc(&iter.pag->pag_ref); + iwag->pag = iter.pag; iwag->iwalk_fn = iwalk_fn; iwag->data = data; iwag->startino = startino; iwag->sz_recs = xfs_iwalk_prefetch(inode_records); iwag->lastino = NULLFSINO; xfs_pwork_queue(&pctl, &iwag->pwork); - startino = XFS_AGINO_TO_INO(mp, pag->pag_agno + 1, 0); + startino = XFS_AGINO_TO_INO(mp, iter.pag->pag_agno + 1, 0); if (flags & XFS_INOBT_WALK_SAME_AG) break; } - if (pag) - xfs_perag_put(pag); if (polled) xfs_pwork_poll(&pctl); return xfs_pwork_destroy(&pctl); @@ -753,30 +747,27 @@ xfs_inobt_walk( .pwork = XFS_PWORK_SINGLE_THREADED, .lastino = NULLFSINO, }; - struct xfs_perag *pag; - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, startino); + xfs_agnumber_t start_agno = XFS_INO_TO_AGNO(mp, startino); int error; - ASSERT(agno < mp->m_sb.sb_agcount); + ASSERT(start_agno < mp->m_sb.sb_agcount); ASSERT(!(flags & ~XFS_INOBT_WALK_FLAGS_ALL)); error = xfs_iwalk_alloc(&iwag); if (error) return error; - for_each_perag_from(mp, agno, pag) { - iwag.pag = pag; + for_each_perag_from(mp, iter, start_agno) { + iwag.pag = iter.pag; error = xfs_iwalk_ag(&iwag); if (error) break; - iwag.startino = XFS_AGINO_TO_INO(mp, pag->pag_agno + 1, 0); + iwag.startino = XFS_AGINO_TO_INO(mp, iter.pag->pag_agno + 1, 0); if (flags & XFS_INOBT_WALK_SAME_AG) break; iwag.pag = NULL; } - if (iwag.pag) - xfs_perag_put(pag); xfs_iwalk_free(&iwag); return error; } diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index a98d2429d795..d51c03dbb01b 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2745,16 +2745,14 @@ xlog_recover_process_iunlinks( struct xlog *log) { struct xfs_mount *mp = log->l_mp; - struct xfs_perag *pag; - xfs_agnumber_t agno; struct xfs_agi *agi; struct xfs_buf *agibp; xfs_agino_t agino; int bucket; int error; - for_each_perag(mp, agno, pag) { - error = xfs_read_agi(mp, NULL, pag->pag_agno, &agibp); + for_each_perag(mp, iter) { + error = xfs_read_agi(mp, NULL, iter.pag->pag_agno, &agibp); if (error) { /* * AGI is b0rked. Don't process it. @@ -2780,7 +2778,8 @@ xlog_recover_process_iunlinks( agino = be32_to_cpu(agi->agi_unlinked[bucket]); while (agino != NULLAGINO) { agino = xlog_recover_process_one_iunlink(mp, - pag->pag_agno, agino, bucket); + iter.pag->pag_agno, agino, + bucket); cond_resched(); } } @@ -3503,10 +3502,8 @@ xlog_recover_check_summary( struct xlog *log) { struct xfs_mount *mp = log->l_mp; - struct xfs_perag *pag; struct xfs_buf *agfbp; struct xfs_buf *agibp; - xfs_agnumber_t agno; uint64_t freeblks; uint64_t itotal; uint64_t ifree; @@ -3517,11 +3514,11 @@ xlog_recover_check_summary( freeblks = 0LL; itotal = 0LL; ifree = 0LL; - for_each_perag(mp, agno, pag) { - error = xfs_read_agf(mp, NULL, pag->pag_agno, 0, &agfbp); + for_each_perag(mp, iter) { + error = xfs_read_agf(mp, NULL, iter.pag->pag_agno, 0, &agfbp); if (error) { xfs_alert(mp, "%s agf read failed agno %d error %d", - __func__, pag->pag_agno, error); + __func__, iter.pag->pag_agno, error); } else { struct xfs_agf *agfp = agfbp->b_addr; @@ -3530,10 +3527,10 @@ xlog_recover_check_summary( xfs_buf_relse(agfbp); } - error = xfs_read_agi(mp, NULL, pag->pag_agno, &agibp); + error = xfs_read_agi(mp, NULL, iter.pag->pag_agno, &agibp); if (error) { xfs_alert(mp, "%s agi read failed agno %d error %d", - __func__, pag->pag_agno, error); + __func__, iter.pag->pag_agno, error); } else { struct xfs_agi *agi = agibp->b_addr; diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index c256104772cb..bc19291c577d 100644 --- a/fs/xfs/xfs_reflink.c +++ b/fs/xfs/xfs_reflink.c @@ -755,19 +755,15 @@ int xfs_reflink_recover_cow( struct xfs_mount *mp) { - struct xfs_perag *pag; - xfs_agnumber_t agno; int error = 0; if (!xfs_sb_version_hasreflink(&mp->m_sb)) return 0; - for_each_perag(mp, agno, pag) { - error = xfs_refcount_recover_cow_leftovers(mp, pag); - if (error) { - xfs_perag_put(pag); + for_each_perag(mp, iter) { + error = xfs_refcount_recover_cow_leftovers(mp, iter.pag); + if (error) break; - } } return error; ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] xfs: automatic resource cleanup of for_each_perag* 2021-08-05 7:01 ` [PATCH 3/5] xfs: automatic resource cleanup of for_each_perag* Darrick J. Wong @ 2021-08-06 7:15 ` Chandan Babu R 2021-08-09 15:07 ` Christoph Hellwig 1 sibling, 0 replies; 15+ messages in thread From: Chandan Babu R @ 2021-08-06 7:15 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On 05 Aug 2021 at 00:01, "Darrick J. Wong" <djwong@kernel.org> wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Jan Kara pointed me to a magic gcc attribute that schedules a finalizer > function to run against any automatic variable that's going out of > scope. Provided nobody minds compiling XFS with gnu99 instead of c89, > we can use this capability to define a loop-scope iterator variable that > will trigger the automatic finalizer, which means that we don't have to > burden for_each_perag* callers with the necessity of manually putting > the perag structures when exiting the loop body. > The behaviour of the code before and after the changes are applied looks to be the same. Hence, Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/Makefile | 3 ++ > fs/xfs/libxfs/xfs_ag.h | 66 +++++++++++++++++++++++++++++---------------- > fs/xfs/libxfs/xfs_sb.c | 17 ++++++------ > fs/xfs/libxfs/xfs_types.c | 6 +--- > fs/xfs/scrub/bmap.c | 8 +---- > fs/xfs/scrub/fscounters.c | 41 ++++++++++++---------------- > fs/xfs/xfs_extent_busy.c | 11 +++----- > fs/xfs/xfs_fsmap.c | 25 ++++++----------- > fs/xfs/xfs_fsops.c | 12 +++----- > fs/xfs/xfs_health.c | 9 +++--- > fs/xfs/xfs_icache.c | 27 ++++++------------ > fs/xfs/xfs_iwalk.c | 41 +++++++++++----------------- > fs/xfs/xfs_log_recover.c | 21 ++++++-------- > fs/xfs/xfs_reflink.c | 10 ++----- > 14 files changed, 135 insertions(+), 162 deletions(-) > > > diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile > index 04611a1068b4..e482cb43743d 100644 > --- a/fs/xfs/Makefile > +++ b/fs/xfs/Makefile > @@ -7,6 +7,9 @@ > ccflags-y += -I $(srctree)/$(src) # needed for trace events > ccflags-y += -I $(srctree)/$(src)/libxfs > > +# Required for for_each_perag* > +ccflags-y += -std=gnu99 > + > obj-$(CONFIG_XFS_FS) += xfs.o > > # this one should be compiled first, as the tracing macros can easily blow up > diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h > index 4c6f9045baca..5141e8936912 100644 > --- a/fs/xfs/libxfs/xfs_ag.h > +++ b/fs/xfs/libxfs/xfs_ag.h > @@ -116,35 +116,53 @@ void xfs_perag_put(struct xfs_perag *pag); > > /* > * Perag iteration APIs > - * > - * XXX: for_each_perag_range() usage really needs an iterator to clean up when > - * we terminate at end_agno because we may have taken a reference to the perag > - * beyond end_agno. Right now callers have to be careful to catch and clean that > - * up themselves. This is not necessary for the callers of for_each_perag() and > - * for_each_perag_from() because they terminate at sb_agcount where there are > - * no perag structures in tree beyond end_agno. > */ > -#define for_each_perag_range(mp, next_agno, end_agno, pag) \ > - for ((pag) = xfs_perag_get((mp), (next_agno)); \ > - (pag) != NULL && (next_agno) <= (end_agno); \ > - (next_agno) = (pag)->pag_agno + 1, \ > - xfs_perag_put(pag), \ > - (pag) = xfs_perag_get((mp), (next_agno))) > +struct xfs_perag_iter { > + struct xfs_perag *pag; > + xfs_agnumber_t __agno; > +}; > > -#define for_each_perag_from(mp, next_agno, pag) \ > - for_each_perag_range((mp), (next_agno), (mp)->m_sb.sb_agcount, (pag)) > +static inline void > +xfs_perag_iter_cleanup(struct xfs_perag_iter *pagit) > +{ > + if (pagit->pag) > + xfs_perag_put(pagit->pag); > + pagit->pag = NULL; > + pagit->__agno = NULLAGNUMBER; > +} > > +#define XFS_PERAG_ITER_INIT(mp, name, first_agno) \ > + struct xfs_perag_iter name \ > + __attribute__((__cleanup__(xfs_perag_iter_cleanup))) = \ > + { .__agno = (first_agno), \ > + .pag = xfs_perag_get((mp), (first_agno)), } > > -#define for_each_perag(mp, agno, pag) \ > - (agno) = 0; \ > - for_each_perag_from((mp), (agno), (pag)) > +#define XFS_PERAG_TAG_ITER_INIT(mp, name, first_agno, tag) \ > + struct xfs_perag_iter name \ > + __attribute__((__cleanup__(xfs_perag_iter_cleanup))) = \ > + { .__agno = (first_agno), \ > + .pag = xfs_perag_get_tag((mp), (first_agno), (tag)), } > > -#define for_each_perag_tag(mp, agno, pag, tag) \ > - for ((agno) = 0, (pag) = xfs_perag_get_tag((mp), 0, (tag)); \ > - (pag) != NULL; \ > - (agno) = (pag)->pag_agno + 1, \ > - xfs_perag_put(pag), \ > - (pag) = xfs_perag_get_tag((mp), (agno), (tag))) > +/* Perag iteration APIs */ > +#define for_each_perag_range(mp, name, start_agno, end_agno) \ > + for (XFS_PERAG_ITER_INIT((mp), (name), (start_agno)); \ > + (name).pag != NULL && (name).__agno <= (end_agno); \ > + (name).__agno = (name).pag->pag_agno + 1, \ > + xfs_perag_put((name).pag), \ > + (name).pag = xfs_perag_get((mp), (name).__agno)) > + > +#define for_each_perag_from(mp, name, start_agno) \ > + for_each_perag_range((mp), (name), (start_agno), (mp)->m_sb.sb_agcount) > + > +#define for_each_perag(mp, name) \ > + for_each_perag_from((mp), (name), 0) > + > +#define for_each_perag_tag(mp, name, tag) \ > + for (XFS_PERAG_TAG_ITER_INIT((mp), (name), 0, (tag)); \ > + (name).pag != NULL; \ > + (name).__agno = (name).pag->pag_agno + 1, \ > + xfs_perag_put((name).pag), \ > + (name).pag = xfs_perag_get_tag((mp), (name).__agno, (tag))) > > struct aghdr_init_data { > /* per ag data */ > diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c > index 04f5386446db..a0a07ebaf41b 100644 > --- a/fs/xfs/libxfs/xfs_sb.c > +++ b/fs/xfs/libxfs/xfs_sb.c > @@ -856,18 +856,19 @@ int > xfs_update_secondary_sbs( > struct xfs_mount *mp) > { > - struct xfs_perag *pag; > - xfs_agnumber_t agno = 1; > + xfs_agnumber_t last_agno = 0; > int saved_error = 0; > int error = 0; > LIST_HEAD (buffer_list); > > /* update secondary superblocks. */ > - for_each_perag_from(mp, agno, pag) { > + for_each_perag_from(mp, iter, 1) { > struct xfs_buf *bp; > > + last_agno = iter.pag->pag_agno; > + > error = xfs_buf_get(mp->m_ddev_targp, > - XFS_AG_DADDR(mp, pag->pag_agno, XFS_SB_DADDR), > + XFS_AG_DADDR(mp, last_agno, XFS_SB_DADDR), > XFS_FSS_TO_BB(mp, 1), &bp); > /* > * If we get an error reading or writing alternate superblocks, > @@ -879,7 +880,7 @@ xfs_update_secondary_sbs( > if (error) { > xfs_warn(mp, > "error allocating secondary superblock for ag %d", > - pag->pag_agno); > + last_agno); > if (!saved_error) > saved_error = error; > continue; > @@ -893,14 +894,14 @@ xfs_update_secondary_sbs( > xfs_buf_relse(bp); > > /* don't hold too many buffers at once */ > - if (agno % 16) > + if (last_agno % 16) > continue; > > error = xfs_buf_delwri_submit(&buffer_list); > if (error) { > xfs_warn(mp, > "write error %d updating a secondary superblock near ag %d", > - error, pag->pag_agno); > + error, last_agno); > if (!saved_error) > saved_error = error; > continue; > @@ -910,7 +911,7 @@ xfs_update_secondary_sbs( > if (error) { > xfs_warn(mp, > "write error %d updating a secondary superblock near ag %d", > - error, agno); > + error, last_agno); > } > > return saved_error ? saved_error : error; > diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c > index e8f4abee7892..8654b46b7c60 100644 > --- a/fs/xfs/libxfs/xfs_types.c > +++ b/fs/xfs/libxfs/xfs_types.c > @@ -223,16 +223,14 @@ xfs_icount_range( > unsigned long long *max) > { > unsigned long long nr_inos = 0; > - struct xfs_perag *pag; > - xfs_agnumber_t agno; > > /* root, rtbitmap, rtsum all live in the first chunk */ > *min = XFS_INODES_PER_CHUNK; > > - for_each_perag(mp, agno, pag) { > + for_each_perag(mp, iter) { > xfs_agino_t first, last; > > - xfs_agino_range(mp, agno, &first, &last); > + xfs_agino_range(mp, iter.pag->pag_agno, &first, &last); > nr_inos += last - first + 1; > } > *max = nr_inos; > diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c > index 1d146c9d9de1..14e952d116f4 100644 > --- a/fs/xfs/scrub/bmap.c > +++ b/fs/xfs/scrub/bmap.c > @@ -576,8 +576,6 @@ xchk_bmap_check_rmaps( > int whichfork) > { > struct xfs_ifork *ifp = XFS_IFORK_PTR(sc->ip, whichfork); > - struct xfs_perag *pag; > - xfs_agnumber_t agno; > bool zero_size; > int error; > > @@ -609,15 +607,13 @@ xchk_bmap_check_rmaps( > (zero_size || ifp->if_nextents > 0)) > return 0; > > - for_each_perag(sc->mp, agno, pag) { > - error = xchk_bmap_check_ag_rmaps(sc, whichfork, pag); > + for_each_perag(sc->mp, iter) { > + error = xchk_bmap_check_ag_rmaps(sc, whichfork, iter.pag); > if (error) > break; > if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT) > break; > } > - if (pag) > - xfs_perag_put(pag); > return error; > } > > diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c > index fd7941e04ae1..e03577af63d9 100644 > --- a/fs/xfs/scrub/fscounters.c > +++ b/fs/xfs/scrub/fscounters.c > @@ -67,21 +67,21 @@ xchk_fscount_warmup( > struct xfs_mount *mp = sc->mp; > struct xfs_buf *agi_bp = NULL; > struct xfs_buf *agf_bp = NULL; > - struct xfs_perag *pag = NULL; > - xfs_agnumber_t agno; > int error = 0; > > - for_each_perag(mp, agno, pag) { > + for_each_perag(mp, iter) { > if (xchk_should_terminate(sc, &error)) > break; > - if (pag->pagi_init && pag->pagf_init) > + if (iter.pag->pagi_init && iter.pag->pagf_init) > continue; > > /* Lock both AG headers. */ > - error = xfs_ialloc_read_agi(mp, sc->tp, agno, &agi_bp); > + error = xfs_ialloc_read_agi(mp, sc->tp, iter.pag->pag_agno, > + &agi_bp); > if (error) > break; > - error = xfs_alloc_read_agf(mp, sc->tp, agno, 0, &agf_bp); > + error = xfs_alloc_read_agf(mp, sc->tp, iter.pag->pag_agno, 0, > + &agf_bp); > if (error) > break; > > @@ -89,7 +89,7 @@ xchk_fscount_warmup( > * These are supposed to be initialized by the header read > * function. > */ > - if (!pag->pagi_init || !pag->pagf_init) { > + if (!iter.pag->pagi_init || !iter.pag->pagf_init) { > error = -EFSCORRUPTED; > break; > } > @@ -104,8 +104,6 @@ xchk_fscount_warmup( > xfs_buf_relse(agf_bp); > if (agi_bp) > xfs_buf_relse(agi_bp); > - if (pag) > - xfs_perag_put(pag); > return error; > } > > @@ -179,9 +177,7 @@ xchk_fscount_aggregate_agcounts( > struct xchk_fscounters *fsc) > { > struct xfs_mount *mp = sc->mp; > - struct xfs_perag *pag; > uint64_t delayed; > - xfs_agnumber_t agno; > int tries = 8; > int error = 0; > > @@ -190,27 +186,28 @@ xchk_fscount_aggregate_agcounts( > fsc->ifree = 0; > fsc->fdblocks = 0; > > - for_each_perag(mp, agno, pag) { > + for_each_perag(mp, iter) { > if (xchk_should_terminate(sc, &error)) > break; > > /* This somehow got unset since the warmup? */ > - if (!pag->pagi_init || !pag->pagf_init) { > + if (!iter.pag->pagi_init || !iter.pag->pagf_init) { > error = -EFSCORRUPTED; > break; > } > > /* Count all the inodes */ > - fsc->icount += pag->pagi_count; > - fsc->ifree += pag->pagi_freecount; > + fsc->icount += iter.pag->pagi_count; > + fsc->ifree += iter.pag->pagi_freecount; > > /* Add up the free/freelist/bnobt/cntbt blocks */ > - fsc->fdblocks += pag->pagf_freeblks; > - fsc->fdblocks += pag->pagf_flcount; > + fsc->fdblocks += iter.pag->pagf_freeblks; > + fsc->fdblocks += iter.pag->pagf_flcount; > if (xfs_sb_version_haslazysbcount(&sc->mp->m_sb)) { > - fsc->fdblocks += pag->pagf_btreeblks; > + fsc->fdblocks += iter.pag->pagf_btreeblks; > } else { > - error = xchk_fscount_btreeblks(sc, fsc, agno); > + error = xchk_fscount_btreeblks(sc, fsc, > + iter.pag->pag_agno); > if (error) > break; > } > @@ -219,12 +216,10 @@ xchk_fscount_aggregate_agcounts( > * Per-AG reservations are taken out of the incore counters, > * so they must be left out of the free blocks computation. > */ > - fsc->fdblocks -= pag->pag_meta_resv.ar_reserved; > - fsc->fdblocks -= pag->pag_rmapbt_resv.ar_orig_reserved; > + fsc->fdblocks -= iter.pag->pag_meta_resv.ar_reserved; > + fsc->fdblocks -= iter.pag->pag_rmapbt_resv.ar_orig_reserved; > > } > - if (pag) > - xfs_perag_put(pag); > if (error) > return error; > > diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c > index ad22a003f959..fe85df915707 100644 > --- a/fs/xfs/xfs_extent_busy.c > +++ b/fs/xfs/xfs_extent_busy.c > @@ -593,18 +593,17 @@ void > xfs_extent_busy_wait_all( > struct xfs_mount *mp) > { > - struct xfs_perag *pag; > DEFINE_WAIT (wait); > - xfs_agnumber_t agno; > > - for_each_perag(mp, agno, pag) { > + for_each_perag(mp, iter) { > do { > - prepare_to_wait(&pag->pagb_wait, &wait, TASK_KILLABLE); > - if (RB_EMPTY_ROOT(&pag->pagb_tree)) > + prepare_to_wait(&iter.pag->pagb_wait, &wait, > + TASK_KILLABLE); > + if (RB_EMPTY_ROOT(&iter.pag->pagb_tree)) > break; > schedule(); > } while (1); > - finish_wait(&pag->pagb_wait, &wait); > + finish_wait(&iter.pag->pagb_wait, &wait); > } > } > > diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c > index 7d0b09c1366e..cdf44db29973 100644 > --- a/fs/xfs/xfs_fsmap.c > +++ b/fs/xfs/xfs_fsmap.c > @@ -573,7 +573,6 @@ __xfs_getfsmap_datadev( > void *priv) > { > struct xfs_mount *mp = tp->t_mountp; > - struct xfs_perag *pag; > struct xfs_btree_cur *bt_cur = NULL; > xfs_fsblock_t start_fsb; > xfs_fsblock_t end_fsb; > @@ -612,13 +611,13 @@ __xfs_getfsmap_datadev( > start_ag = XFS_FSB_TO_AGNO(mp, start_fsb); > end_ag = XFS_FSB_TO_AGNO(mp, end_fsb); > > - for_each_perag_range(mp, start_ag, end_ag, pag) { > + for_each_perag_range(mp, iter, start_ag, end_ag) { > /* > * Set the AG high key from the fsmap high key if this > * is the last AG that we're querying. > */ > - info->pag = pag; > - if (pag->pag_agno == end_ag) { > + info->pag = iter.pag; > + if (iter.pag->pag_agno == end_ag) { > info->high.rm_startblock = XFS_FSB_TO_AGBNO(mp, > end_fsb); > info->high.rm_offset = XFS_BB_TO_FSBT(mp, > @@ -636,14 +635,14 @@ __xfs_getfsmap_datadev( > info->agf_bp = NULL; > } > > - error = xfs_alloc_read_agf(mp, tp, pag->pag_agno, 0, > + error = xfs_alloc_read_agf(mp, tp, iter.pag->pag_agno, 0, > &info->agf_bp); > if (error) > break; > > - trace_xfs_fsmap_low_key(mp, info->dev, pag->pag_agno, > + trace_xfs_fsmap_low_key(mp, info->dev, iter.pag->pag_agno, > &info->low); > - trace_xfs_fsmap_high_key(mp, info->dev, pag->pag_agno, > + trace_xfs_fsmap_high_key(mp, info->dev, iter.pag->pag_agno, > &info->high); > > error = query_fn(tp, info, &bt_cur, priv); > @@ -654,7 +653,7 @@ __xfs_getfsmap_datadev( > * Set the AG low key to the start of the AG prior to > * moving on to the next AG. > */ > - if (pag->pag_agno == start_ag) { > + if (iter.pag->pag_agno == start_ag) { > info->low.rm_startblock = 0; > info->low.rm_owner = 0; > info->low.rm_offset = 0; > @@ -666,7 +665,7 @@ __xfs_getfsmap_datadev( > * before we drop the reference to the perag when the loop > * terminates. > */ > - if (pag->pag_agno == end_ag) { > + if (iter.pag->pag_agno == end_ag) { > info->last = true; > error = query_fn(tp, info, &bt_cur, priv); > if (error) > @@ -682,13 +681,7 @@ __xfs_getfsmap_datadev( > xfs_trans_brelse(tp, info->agf_bp); > info->agf_bp = NULL; > } > - if (info->pag) { > - xfs_perag_put(info->pag); > - info->pag = NULL; > - } else if (pag) { > - /* loop termination case */ > - xfs_perag_put(pag); > - } > + info->pag = NULL; > > return error; > } > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c > index 6ed29b158312..31f585a525e8 100644 > --- a/fs/xfs/xfs_fsops.c > +++ b/fs/xfs/xfs_fsops.c > @@ -570,14 +570,12 @@ int > xfs_fs_reserve_ag_blocks( > struct xfs_mount *mp) > { > - xfs_agnumber_t agno; > - struct xfs_perag *pag; > int error = 0; > int err2; > > mp->m_finobt_nores = false; > - for_each_perag(mp, agno, pag) { > - err2 = xfs_ag_resv_init(pag, NULL); > + for_each_perag(mp, iter) { > + err2 = xfs_ag_resv_init(iter.pag, NULL); > if (err2 && !error) > error = err2; > } > @@ -598,13 +596,11 @@ int > xfs_fs_unreserve_ag_blocks( > struct xfs_mount *mp) > { > - xfs_agnumber_t agno; > - struct xfs_perag *pag; > int error = 0; > int err2; > > - for_each_perag(mp, agno, pag) { > - err2 = xfs_ag_resv_free(pag); > + for_each_perag(mp, iter) { > + err2 = xfs_ag_resv_free(iter.pag); > if (err2 && !error) > error = err2; > } > diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c > index eb10eacabc8f..715a987b8204 100644 > --- a/fs/xfs/xfs_health.c > +++ b/fs/xfs/xfs_health.c > @@ -24,8 +24,6 @@ void > xfs_health_unmount( > struct xfs_mount *mp) > { > - struct xfs_perag *pag; > - xfs_agnumber_t agno; > unsigned int sick = 0; > unsigned int checked = 0; > bool warn = false; > @@ -34,10 +32,11 @@ xfs_health_unmount( > return; > > /* Measure AG corruption levels. */ > - for_each_perag(mp, agno, pag) { > - xfs_ag_measure_sickness(pag, &sick, &checked); > + for_each_perag(mp, iter) { > + xfs_ag_measure_sickness(iter.pag, &sick, &checked); > if (sick) { > - trace_xfs_ag_unfixed_corruption(mp, agno, sick); > + trace_xfs_ag_unfixed_corruption(mp, iter.pag->pag_agno, > + sick); > warn = true; > } > } > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c > index 6741e27603ad..1d4006df4066 100644 > --- a/fs/xfs/xfs_icache.c > +++ b/fs/xfs/xfs_icache.c > @@ -1368,14 +1368,11 @@ void > xfs_blockgc_stop( > struct xfs_mount *mp) > { > - struct xfs_perag *pag; > - xfs_agnumber_t agno; > - > if (!xfs_clear_blockgc_enabled(mp)) > return; > > - for_each_perag(mp, agno, pag) > - cancel_delayed_work_sync(&pag->pag_blockgc_work); > + for_each_perag(mp, iter) > + cancel_delayed_work_sync(&iter.pag->pag_blockgc_work); > trace_xfs_blockgc_stop(mp, __return_address); > } > > @@ -1384,15 +1381,12 @@ void > xfs_blockgc_start( > struct xfs_mount *mp) > { > - struct xfs_perag *pag; > - xfs_agnumber_t agno; > - > if (xfs_set_blockgc_enabled(mp)) > return; > > trace_xfs_blockgc_start(mp, __return_address); > - for_each_perag_tag(mp, agno, pag, XFS_ICI_BLOCKGC_TAG) > - xfs_blockgc_queue(pag); > + for_each_perag_tag(mp, iter, XFS_ICI_BLOCKGC_TAG) > + xfs_blockgc_queue(iter.pag); > } > > /* Don't try to run block gc on an inode that's in any of these states. */ > @@ -1515,9 +1509,6 @@ void > xfs_blockgc_flush_all( > struct xfs_mount *mp) > { > - struct xfs_perag *pag; > - xfs_agnumber_t agno; > - > trace_xfs_blockgc_flush_all(mp, __return_address); > > /* > @@ -1525,12 +1516,12 @@ xfs_blockgc_flush_all( > * wasn't queued, it will not be requeued. Then flush whatever's > * left. > */ > - for_each_perag_tag(mp, agno, pag, XFS_ICI_BLOCKGC_TAG) > - mod_delayed_work(pag->pag_mount->m_blockgc_wq, > - &pag->pag_blockgc_work, 0); > + for_each_perag_tag(mp, iter, XFS_ICI_BLOCKGC_TAG) > + mod_delayed_work(mp->m_blockgc_wq, &iter.pag->pag_blockgc_work, > + 0); > > - for_each_perag_tag(mp, agno, pag, XFS_ICI_BLOCKGC_TAG) > - flush_delayed_work(&pag->pag_blockgc_work); > + for_each_perag_tag(mp, iter, XFS_ICI_BLOCKGC_TAG) > + flush_delayed_work(&iter.pag->pag_blockgc_work); > > xfs_inodegc_flush(mp); > } > diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c > index 7558486f4937..0c06a59aec79 100644 > --- a/fs/xfs/xfs_iwalk.c > +++ b/fs/xfs/xfs_iwalk.c > @@ -568,30 +568,27 @@ xfs_iwalk( > .pwork = XFS_PWORK_SINGLE_THREADED, > .lastino = NULLFSINO, > }; > - struct xfs_perag *pag; > - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, startino); > + xfs_agnumber_t start_agno = XFS_INO_TO_AGNO(mp, startino); > int error; > > - ASSERT(agno < mp->m_sb.sb_agcount); > + ASSERT(start_agno < mp->m_sb.sb_agcount); > ASSERT(!(flags & ~XFS_IWALK_FLAGS_ALL)); > > error = xfs_iwalk_alloc(&iwag); > if (error) > return error; > > - for_each_perag_from(mp, agno, pag) { > - iwag.pag = pag; > + for_each_perag_from(mp, iter, start_agno) { > + iwag.pag = iter.pag; > error = xfs_iwalk_ag(&iwag); > if (error) > break; > - iwag.startino = XFS_AGINO_TO_INO(mp, agno + 1, 0); > + iwag.startino = XFS_AGINO_TO_INO(mp, iter.pag->pag_agno + 1, 0); > if (flags & XFS_INOBT_WALK_SAME_AG) > break; > iwag.pag = NULL; > } > > - if (iwag.pag) > - xfs_perag_put(pag); > xfs_iwalk_free(&iwag); > return error; > } > @@ -646,18 +643,17 @@ xfs_iwalk_threaded( > void *data) > { > struct xfs_pwork_ctl pctl; > - struct xfs_perag *pag; > - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, startino); > + xfs_agnumber_t start_agno = XFS_INO_TO_AGNO(mp, startino); > int error; > > - ASSERT(agno < mp->m_sb.sb_agcount); > + ASSERT(start_agno < mp->m_sb.sb_agcount); > ASSERT(!(flags & ~XFS_IWALK_FLAGS_ALL)); > > error = xfs_pwork_init(mp, &pctl, xfs_iwalk_ag_work, "xfs_iwalk"); > if (error) > return error; > > - for_each_perag_from(mp, agno, pag) { > + for_each_perag_from(mp, iter, start_agno) { > struct xfs_iwalk_ag *iwag; > > if (xfs_pwork_ctl_want_abort(&pctl)) > @@ -670,20 +666,18 @@ xfs_iwalk_threaded( > * perag is being handed off to async work, so take another > * reference for the async work to release. > */ > - atomic_inc(&pag->pag_ref); > - iwag->pag = pag; > + atomic_inc(&iter.pag->pag_ref); > + iwag->pag = iter.pag; > iwag->iwalk_fn = iwalk_fn; > iwag->data = data; > iwag->startino = startino; > iwag->sz_recs = xfs_iwalk_prefetch(inode_records); > iwag->lastino = NULLFSINO; > xfs_pwork_queue(&pctl, &iwag->pwork); > - startino = XFS_AGINO_TO_INO(mp, pag->pag_agno + 1, 0); > + startino = XFS_AGINO_TO_INO(mp, iter.pag->pag_agno + 1, 0); > if (flags & XFS_INOBT_WALK_SAME_AG) > break; > } > - if (pag) > - xfs_perag_put(pag); > if (polled) > xfs_pwork_poll(&pctl); > return xfs_pwork_destroy(&pctl); > @@ -753,30 +747,27 @@ xfs_inobt_walk( > .pwork = XFS_PWORK_SINGLE_THREADED, > .lastino = NULLFSINO, > }; > - struct xfs_perag *pag; > - xfs_agnumber_t agno = XFS_INO_TO_AGNO(mp, startino); > + xfs_agnumber_t start_agno = XFS_INO_TO_AGNO(mp, startino); > int error; > > - ASSERT(agno < mp->m_sb.sb_agcount); > + ASSERT(start_agno < mp->m_sb.sb_agcount); > ASSERT(!(flags & ~XFS_INOBT_WALK_FLAGS_ALL)); > > error = xfs_iwalk_alloc(&iwag); > if (error) > return error; > > - for_each_perag_from(mp, agno, pag) { > - iwag.pag = pag; > + for_each_perag_from(mp, iter, start_agno) { > + iwag.pag = iter.pag; > error = xfs_iwalk_ag(&iwag); > if (error) > break; > - iwag.startino = XFS_AGINO_TO_INO(mp, pag->pag_agno + 1, 0); > + iwag.startino = XFS_AGINO_TO_INO(mp, iter.pag->pag_agno + 1, 0); > if (flags & XFS_INOBT_WALK_SAME_AG) > break; > iwag.pag = NULL; > } > > - if (iwag.pag) > - xfs_perag_put(pag); > xfs_iwalk_free(&iwag); > return error; > } > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index a98d2429d795..d51c03dbb01b 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -2745,16 +2745,14 @@ xlog_recover_process_iunlinks( > struct xlog *log) > { > struct xfs_mount *mp = log->l_mp; > - struct xfs_perag *pag; > - xfs_agnumber_t agno; > struct xfs_agi *agi; > struct xfs_buf *agibp; > xfs_agino_t agino; > int bucket; > int error; > > - for_each_perag(mp, agno, pag) { > - error = xfs_read_agi(mp, NULL, pag->pag_agno, &agibp); > + for_each_perag(mp, iter) { > + error = xfs_read_agi(mp, NULL, iter.pag->pag_agno, &agibp); > if (error) { > /* > * AGI is b0rked. Don't process it. > @@ -2780,7 +2778,8 @@ xlog_recover_process_iunlinks( > agino = be32_to_cpu(agi->agi_unlinked[bucket]); > while (agino != NULLAGINO) { > agino = xlog_recover_process_one_iunlink(mp, > - pag->pag_agno, agino, bucket); > + iter.pag->pag_agno, agino, > + bucket); > cond_resched(); > } > } > @@ -3503,10 +3502,8 @@ xlog_recover_check_summary( > struct xlog *log) > { > struct xfs_mount *mp = log->l_mp; > - struct xfs_perag *pag; > struct xfs_buf *agfbp; > struct xfs_buf *agibp; > - xfs_agnumber_t agno; > uint64_t freeblks; > uint64_t itotal; > uint64_t ifree; > @@ -3517,11 +3514,11 @@ xlog_recover_check_summary( > freeblks = 0LL; > itotal = 0LL; > ifree = 0LL; > - for_each_perag(mp, agno, pag) { > - error = xfs_read_agf(mp, NULL, pag->pag_agno, 0, &agfbp); > + for_each_perag(mp, iter) { > + error = xfs_read_agf(mp, NULL, iter.pag->pag_agno, 0, &agfbp); > if (error) { > xfs_alert(mp, "%s agf read failed agno %d error %d", > - __func__, pag->pag_agno, error); > + __func__, iter.pag->pag_agno, error); > } else { > struct xfs_agf *agfp = agfbp->b_addr; > > @@ -3530,10 +3527,10 @@ xlog_recover_check_summary( > xfs_buf_relse(agfbp); > } > > - error = xfs_read_agi(mp, NULL, pag->pag_agno, &agibp); > + error = xfs_read_agi(mp, NULL, iter.pag->pag_agno, &agibp); > if (error) { > xfs_alert(mp, "%s agi read failed agno %d error %d", > - __func__, pag->pag_agno, error); > + __func__, iter.pag->pag_agno, error); > } else { > struct xfs_agi *agi = agibp->b_addr; > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > index c256104772cb..bc19291c577d 100644 > --- a/fs/xfs/xfs_reflink.c > +++ b/fs/xfs/xfs_reflink.c > @@ -755,19 +755,15 @@ int > xfs_reflink_recover_cow( > struct xfs_mount *mp) > { > - struct xfs_perag *pag; > - xfs_agnumber_t agno; > int error = 0; > > if (!xfs_sb_version_hasreflink(&mp->m_sb)) > return 0; > > - for_each_perag(mp, agno, pag) { > - error = xfs_refcount_recover_cow_leftovers(mp, pag); > - if (error) { > - xfs_perag_put(pag); > + for_each_perag(mp, iter) { > + error = xfs_refcount_recover_cow_leftovers(mp, iter.pag); > + if (error) > break; > - } > } > > return error; -- chandan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] xfs: automatic resource cleanup of for_each_perag* 2021-08-05 7:01 ` [PATCH 3/5] xfs: automatic resource cleanup of for_each_perag* Darrick J. Wong 2021-08-06 7:15 ` Chandan Babu R @ 2021-08-09 15:07 ` Christoph Hellwig 1 sibling, 0 replies; 15+ messages in thread From: Christoph Hellwig @ 2021-08-09 15:07 UTC (permalink / raw) To: Darrick J. Wong Cc: linux-xfs, linux-kernel, ojeda, nathan, ndesaulniers, arnd > +# Required for for_each_perag* > +ccflags-y += -std=gnu99 I don't think it is up to an individual subsystem to pick a specific C dialect. I think the most important reason why the kernel sticks with gnu89 is to avoid the misfeature of variable declarations in the middle of blocks, and this change would lose it. > + xfs_agnumber_t last_agno = 0; > int saved_error = 0; > int error = 0; > LIST_HEAD (buffer_list); > > /* update secondary superblocks. */ > - for_each_perag_from(mp, agno, pag) { > + for_each_perag_from(mp, iter, 1) { > struct xfs_buf *bp; > > + last_agno = iter.pag->pag_agno; This is a really horrible API as it magically injects a local variable in a macro. It also leads to worse code generation and a small but noticable increase in .text sie: hch@brick:~/work/xfs$ size xfs.o.* text data bss dec hex filename 1521421 301161 1880 1824462 1bd6ce xfs.o.old 1521516 301161 1880 1824557 1bd72d xfs.o.new ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/5] xfs: grab active perag ref when reading AG headers 2021-08-05 7:00 [PATCHSET 0/5] xfs: other stuff for 5.15 Darrick J. Wong ` (2 preceding siblings ...) 2021-08-05 7:01 ` [PATCH 3/5] xfs: automatic resource cleanup of for_each_perag* Darrick J. Wong @ 2021-08-05 7:01 ` Darrick J. Wong 2021-08-06 11:25 ` Chandan Babu R 2021-08-05 7:01 ` [PATCH 5/5] xfs: dump log intent items that cannot be recovered due to corruption Darrick J. Wong 4 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2021-08-05 7:01 UTC (permalink / raw) To: djwong; +Cc: linux-xfs From: Darrick J. Wong <djwong@kernel.org> This patch prepares scrub to deal with the possibility of tearing down entire AGs by changing the order of resource acquisition to match the rest of the XFS codebase. In other words, scrub now grabs AG resources in order of: perag structure, then AGI/AGF/AGFL buffers, then btree cursors; and releases them in reverse order. This requires us to distinguish xchk_ag_init callers -- some are responding to a user request to check AG metadata, in which case we can return ENOENT to userspace; but other callers have an ondisk reference to an AG that they're trying to cross-reference. In this second case, the lack of an AG means there's ondisk corruption, since ondisk metadata cannot point into nonexistent space. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- fs/xfs/scrub/agheader.c | 23 +++++++++++++------ fs/xfs/scrub/agheader_repair.c | 3 --- fs/xfs/scrub/bmap.c | 2 +- fs/xfs/scrub/btree.c | 2 +- fs/xfs/scrub/common.c | 48 ++++++++++++++++------------------------ fs/xfs/scrub/common.h | 18 ++++++++++++++- fs/xfs/scrub/fscounters.c | 2 +- fs/xfs/scrub/inode.c | 2 +- 8 files changed, 56 insertions(+), 44 deletions(-) diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c index be1a7e1e65f7..6152ce01c057 100644 --- a/fs/xfs/scrub/agheader.c +++ b/fs/xfs/scrub/agheader.c @@ -36,7 +36,7 @@ xchk_superblock_xref( agbno = XFS_SB_BLOCK(mp); - error = xchk_ag_init(sc, agno, &sc->sa); + error = xchk_ag_init_existing(sc, agno, &sc->sa); if (!xchk_xref_process_error(sc, agno, agbno, &error)) return; @@ -63,6 +63,7 @@ xchk_superblock( struct xfs_mount *mp = sc->mp; struct xfs_buf *bp; struct xfs_dsb *sb; + struct xfs_perag *pag; xfs_agnumber_t agno; uint32_t v2_ok; __be32 features_mask; @@ -73,6 +74,15 @@ xchk_superblock( if (agno == 0) return 0; + /* + * Grab an active reference to the perag structure. If we can't get + * it, we're racing with something that's tearing down the AG, so + * signal that the AG no longer exists. + */ + pag = xfs_perag_get(mp, agno); + if (!pag) + return -ENOENT; + error = xfs_sb_read_secondary(mp, sc->tp, agno, &bp); /* * The superblock verifier can return several different error codes @@ -92,7 +102,7 @@ xchk_superblock( break; } if (!xchk_process_error(sc, agno, XFS_SB_BLOCK(mp), &error)) - return error; + goto out_pag; sb = bp->b_addr; @@ -336,7 +346,8 @@ xchk_superblock( xchk_block_set_corrupt(sc, bp); xchk_superblock_xref(sc, bp); - +out_pag: + xfs_perag_put(pag); return error; } @@ -527,6 +538,7 @@ xchk_agf( xchk_buffer_recheck(sc, sc->sa.agf_bp); agf = sc->sa.agf_bp->b_addr; + pag = sc->sa.pag; /* Check the AG length */ eoag = be32_to_cpu(agf->agf_length); @@ -582,7 +594,6 @@ xchk_agf( xchk_block_set_corrupt(sc, sc->sa.agf_bp); /* Do the incore counters match? */ - pag = xfs_perag_get(mp, agno); if (pag->pagf_freeblks != be32_to_cpu(agf->agf_freeblks)) xchk_block_set_corrupt(sc, sc->sa.agf_bp); if (pag->pagf_flcount != be32_to_cpu(agf->agf_flcount)) @@ -590,7 +601,6 @@ xchk_agf( if (xfs_sb_version_haslazysbcount(&sc->mp->m_sb) && pag->pagf_btreeblks != be32_to_cpu(agf->agf_btreeblks)) xchk_block_set_corrupt(sc, sc->sa.agf_bp); - xfs_perag_put(pag); xchk_agf_xref(sc); out: @@ -857,6 +867,7 @@ xchk_agi( xchk_buffer_recheck(sc, sc->sa.agi_bp); agi = sc->sa.agi_bp->b_addr; + pag = sc->sa.pag; /* Check the AG length */ eoag = be32_to_cpu(agi->agi_length); @@ -909,12 +920,10 @@ xchk_agi( xchk_block_set_corrupt(sc, sc->sa.agi_bp); /* Do the incore counters match? */ - pag = xfs_perag_get(mp, agno); if (pag->pagi_count != be32_to_cpu(agi->agi_count)) xchk_block_set_corrupt(sc, sc->sa.agi_bp); if (pag->pagi_freecount != be32_to_cpu(agi->agi_freecount)) xchk_block_set_corrupt(sc, sc->sa.agi_bp); - xfs_perag_put(pag); xchk_agi_xref(sc); out: diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c index e95f8c98f0f7..f122f2e20e79 100644 --- a/fs/xfs/scrub/agheader_repair.c +++ b/fs/xfs/scrub/agheader_repair.c @@ -366,7 +366,6 @@ xrep_agf( if (!xfs_sb_version_hasrmapbt(&mp->m_sb)) return -EOPNOTSUPP; - xchk_perag_get(sc->mp, &sc->sa); /* * Make sure we have the AGF buffer, as scrub might have decided it * was corrupt after xfs_alloc_read_agf failed with -EFSCORRUPTED. @@ -641,7 +640,6 @@ xrep_agfl( if (!xfs_sb_version_hasrmapbt(&mp->m_sb)) return -EOPNOTSUPP; - xchk_perag_get(sc->mp, &sc->sa); xbitmap_init(&agfl_extents); /* @@ -896,7 +894,6 @@ xrep_agi( if (!xfs_sb_version_hasrmapbt(&mp->m_sb)) return -EOPNOTSUPP; - xchk_perag_get(sc->mp, &sc->sa); /* * Make sure we have the AGI buffer, as scrub might have decided it * was corrupt after xfs_ialloc_read_agi failed with -EFSCORRUPTED. diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c index 14e952d116f4..a5ca2856df8b 100644 --- a/fs/xfs/scrub/bmap.c +++ b/fs/xfs/scrub/bmap.c @@ -260,7 +260,7 @@ xchk_bmap_iextent_xref( agbno = XFS_FSB_TO_AGBNO(mp, irec->br_startblock); len = irec->br_blockcount; - error = xchk_ag_init(info->sc, agno, &info->sc->sa); + error = xchk_ag_init_existing(info->sc, agno, &info->sc->sa); if (!xchk_fblock_process_error(info->sc, info->whichfork, irec->br_startoff, &error)) return; diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c index bd1172358964..c044e0a8da7f 100644 --- a/fs/xfs/scrub/btree.c +++ b/fs/xfs/scrub/btree.c @@ -374,7 +374,7 @@ xchk_btree_check_block_owner( init_sa = bs->cur->bc_flags & XFS_BTREE_LONG_PTRS; if (init_sa) { - error = xchk_ag_init(bs->sc, agno, &bs->sc->sa); + error = xchk_ag_init_existing(bs->sc, agno, &bs->sc->sa); if (!xchk_btree_xref_process_error(bs->sc, bs->cur, level, &error)) return error; diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c index e86854171b0c..0ef96ed71017 100644 --- a/fs/xfs/scrub/common.c +++ b/fs/xfs/scrub/common.c @@ -394,11 +394,11 @@ want_ag_read_header_failure( } /* - * Grab all the headers for an AG. + * Grab the perag structure and all the headers for an AG. * - * The headers should be released by xchk_ag_free, but as a fail - * safe we attach all the buffers we grab to the scrub transaction so - * they'll all be freed when we cancel it. + * The headers should be released by xchk_ag_free, but as a fail safe we attach + * all the buffers we grab to the scrub transaction so they'll all be freed + * when we cancel it. Returns ENOENT if we can't grab the perag structure. */ int xchk_ag_read_headers( @@ -409,22 +409,26 @@ xchk_ag_read_headers( struct xfs_mount *mp = sc->mp; int error; + ASSERT(!sa->pag); + sa->pag = xfs_perag_get(mp, agno); + if (!sa->pag) + return -ENOENT; + sa->agno = agno; error = xfs_ialloc_read_agi(mp, sc->tp, agno, &sa->agi_bp); if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGI)) - goto out; + return error; error = xfs_alloc_read_agf(mp, sc->tp, agno, 0, &sa->agf_bp); if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGF)) - goto out; + return error; error = xfs_alloc_read_agfl(mp, sc->tp, agno, &sa->agfl_bp); if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGFL)) - goto out; - error = 0; -out: - return error; + return error; + + return 0; } /* Release all the AG btree cursors. */ @@ -461,7 +465,6 @@ xchk_ag_btcur_init( { struct xfs_mount *mp = sc->mp; - xchk_perag_get(sc->mp, sa); if (sa->agf_bp && xchk_ag_btree_healthy_enough(sc, sa->pag, XFS_BTNUM_BNO)) { /* Set up a bnobt cursor for cross-referencing. */ @@ -532,11 +535,11 @@ xchk_ag_free( } /* - * For scrub, grab the AGI and the AGF headers, in that order. Locking - * order requires us to get the AGI before the AGF. We use the - * transaction to avoid deadlocking on crosslinked metadata buffers; - * either the caller passes one in (bmap scrub) or we have to create a - * transaction ourselves. + * For scrub, grab the perag structure, the AGI, and the AGF headers, in that + * order. Locking order requires us to get the AGI before the AGF. We use the + * transaction to avoid deadlocking on crosslinked metadata buffers; either the + * caller passes one in (bmap scrub) or we have to create a transaction + * ourselves. Returns ENOENT if the perag struct cannot be grabbed. */ int xchk_ag_init( @@ -554,19 +557,6 @@ xchk_ag_init( return 0; } -/* - * Grab the per-ag structure if we haven't already gotten it. Teardown of the - * xchk_ag will release it for us. - */ -void -xchk_perag_get( - struct xfs_mount *mp, - struct xchk_ag *sa) -{ - if (!sa->pag) - sa->pag = xfs_perag_get(mp, sa->agno); -} - /* Per-scrubber setup functions */ /* diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h index 0410faf7d735..454145db10e7 100644 --- a/fs/xfs/scrub/common.h +++ b/fs/xfs/scrub/common.h @@ -107,7 +107,23 @@ int xchk_setup_fscounters(struct xfs_scrub *sc); void xchk_ag_free(struct xfs_scrub *sc, struct xchk_ag *sa); int xchk_ag_init(struct xfs_scrub *sc, xfs_agnumber_t agno, struct xchk_ag *sa); -void xchk_perag_get(struct xfs_mount *mp, struct xchk_ag *sa); + +/* + * Grab all AG resources, treating the inability to grab the perag structure as + * a fs corruption. This is intended for callers checking an ondisk reference + * to a given AG, which means that the AG must still exist. + */ +static inline int +xchk_ag_init_existing( + struct xfs_scrub *sc, + xfs_agnumber_t agno, + struct xchk_ag *sa) +{ + int error = xchk_ag_init(sc, agno, sa); + + return error == -ENOENT ? -EFSCORRUPTED : error; +} + int xchk_ag_read_headers(struct xfs_scrub *sc, xfs_agnumber_t agno, struct xchk_ag *sa); void xchk_ag_btcur_free(struct xchk_ag *sa); diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c index e03577af63d9..74e331afe49d 100644 --- a/fs/xfs/scrub/fscounters.c +++ b/fs/xfs/scrub/fscounters.c @@ -146,7 +146,7 @@ xchk_fscount_btreeblks( xfs_extlen_t blocks; int error; - error = xchk_ag_init(sc, agno, &sc->sa); + error = xchk_ag_init_existing(sc, agno, &sc->sa); if (error) return error; diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c index 76fbc7ca4cec..a6a68ba19f0a 100644 --- a/fs/xfs/scrub/inode.c +++ b/fs/xfs/scrub/inode.c @@ -532,7 +532,7 @@ xchk_inode_xref( agno = XFS_INO_TO_AGNO(sc->mp, ino); agbno = XFS_INO_TO_AGBNO(sc->mp, ino); - error = xchk_ag_init(sc, agno, &sc->sa); + error = xchk_ag_init_existing(sc, agno, &sc->sa); if (!xchk_xref_process_error(sc, agno, agbno, &error)) return; ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] xfs: grab active perag ref when reading AG headers 2021-08-05 7:01 ` [PATCH 4/5] xfs: grab active perag ref when reading AG headers Darrick J. Wong @ 2021-08-06 11:25 ` Chandan Babu R 0 siblings, 0 replies; 15+ messages in thread From: Chandan Babu R @ 2021-08-06 11:25 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On 05 Aug 2021 at 00:01, "Darrick J. Wong" <djwong@kernel.org> wrote: > From: Darrick J. Wong <djwong@kernel.org> > > This patch prepares scrub to deal with the possibility of tearing down > entire AGs by changing the order of resource acquisition to match the > rest of the XFS codebase. In other words, scrub now grabs AG resources > in order of: perag structure, then AGI/AGF/AGFL buffers, then btree > cursors; and releases them in reverse order. > > This requires us to distinguish xchk_ag_init callers -- some are > responding to a user request to check AG metadata, in which case we can > return ENOENT to userspace; but other callers have an ondisk reference > to an AG that they're trying to cross-reference. In this second case, > the lack of an AG means there's ondisk corruption, since ondisk metadata > cannot point into nonexistent space. > As mentioned above, with this patch applied, scrub code either obtains a reference to a metadata belonging to an AG or obtain a reference to the pag structure during setup phase. Also, a reference to the pag structure is obtained before getting a reference to AGI, AGF and AGFL. Hence, Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/scrub/agheader.c | 23 +++++++++++++------ > fs/xfs/scrub/agheader_repair.c | 3 --- > fs/xfs/scrub/bmap.c | 2 +- > fs/xfs/scrub/btree.c | 2 +- > fs/xfs/scrub/common.c | 48 ++++++++++++++++------------------------ > fs/xfs/scrub/common.h | 18 ++++++++++++++- > fs/xfs/scrub/fscounters.c | 2 +- > fs/xfs/scrub/inode.c | 2 +- > 8 files changed, 56 insertions(+), 44 deletions(-) > > > diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c > index be1a7e1e65f7..6152ce01c057 100644 > --- a/fs/xfs/scrub/agheader.c > +++ b/fs/xfs/scrub/agheader.c > @@ -36,7 +36,7 @@ xchk_superblock_xref( > > agbno = XFS_SB_BLOCK(mp); > > - error = xchk_ag_init(sc, agno, &sc->sa); > + error = xchk_ag_init_existing(sc, agno, &sc->sa); > if (!xchk_xref_process_error(sc, agno, agbno, &error)) > return; > > @@ -63,6 +63,7 @@ xchk_superblock( > struct xfs_mount *mp = sc->mp; > struct xfs_buf *bp; > struct xfs_dsb *sb; > + struct xfs_perag *pag; > xfs_agnumber_t agno; > uint32_t v2_ok; > __be32 features_mask; > @@ -73,6 +74,15 @@ xchk_superblock( > if (agno == 0) > return 0; > > + /* > + * Grab an active reference to the perag structure. If we can't get > + * it, we're racing with something that's tearing down the AG, so > + * signal that the AG no longer exists. > + */ > + pag = xfs_perag_get(mp, agno); > + if (!pag) > + return -ENOENT; > + > error = xfs_sb_read_secondary(mp, sc->tp, agno, &bp); > /* > * The superblock verifier can return several different error codes > @@ -92,7 +102,7 @@ xchk_superblock( > break; > } > if (!xchk_process_error(sc, agno, XFS_SB_BLOCK(mp), &error)) > - return error; > + goto out_pag; > > sb = bp->b_addr; > > @@ -336,7 +346,8 @@ xchk_superblock( > xchk_block_set_corrupt(sc, bp); > > xchk_superblock_xref(sc, bp); > - > +out_pag: > + xfs_perag_put(pag); > return error; > } > > @@ -527,6 +538,7 @@ xchk_agf( > xchk_buffer_recheck(sc, sc->sa.agf_bp); > > agf = sc->sa.agf_bp->b_addr; > + pag = sc->sa.pag; > > /* Check the AG length */ > eoag = be32_to_cpu(agf->agf_length); > @@ -582,7 +594,6 @@ xchk_agf( > xchk_block_set_corrupt(sc, sc->sa.agf_bp); > > /* Do the incore counters match? */ > - pag = xfs_perag_get(mp, agno); > if (pag->pagf_freeblks != be32_to_cpu(agf->agf_freeblks)) > xchk_block_set_corrupt(sc, sc->sa.agf_bp); > if (pag->pagf_flcount != be32_to_cpu(agf->agf_flcount)) > @@ -590,7 +601,6 @@ xchk_agf( > if (xfs_sb_version_haslazysbcount(&sc->mp->m_sb) && > pag->pagf_btreeblks != be32_to_cpu(agf->agf_btreeblks)) > xchk_block_set_corrupt(sc, sc->sa.agf_bp); > - xfs_perag_put(pag); > > xchk_agf_xref(sc); > out: > @@ -857,6 +867,7 @@ xchk_agi( > xchk_buffer_recheck(sc, sc->sa.agi_bp); > > agi = sc->sa.agi_bp->b_addr; > + pag = sc->sa.pag; > > /* Check the AG length */ > eoag = be32_to_cpu(agi->agi_length); > @@ -909,12 +920,10 @@ xchk_agi( > xchk_block_set_corrupt(sc, sc->sa.agi_bp); > > /* Do the incore counters match? */ > - pag = xfs_perag_get(mp, agno); > if (pag->pagi_count != be32_to_cpu(agi->agi_count)) > xchk_block_set_corrupt(sc, sc->sa.agi_bp); > if (pag->pagi_freecount != be32_to_cpu(agi->agi_freecount)) > xchk_block_set_corrupt(sc, sc->sa.agi_bp); > - xfs_perag_put(pag); > > xchk_agi_xref(sc); > out: > diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c > index e95f8c98f0f7..f122f2e20e79 100644 > --- a/fs/xfs/scrub/agheader_repair.c > +++ b/fs/xfs/scrub/agheader_repair.c > @@ -366,7 +366,6 @@ xrep_agf( > if (!xfs_sb_version_hasrmapbt(&mp->m_sb)) > return -EOPNOTSUPP; > > - xchk_perag_get(sc->mp, &sc->sa); > /* > * Make sure we have the AGF buffer, as scrub might have decided it > * was corrupt after xfs_alloc_read_agf failed with -EFSCORRUPTED. > @@ -641,7 +640,6 @@ xrep_agfl( > if (!xfs_sb_version_hasrmapbt(&mp->m_sb)) > return -EOPNOTSUPP; > > - xchk_perag_get(sc->mp, &sc->sa); > xbitmap_init(&agfl_extents); > > /* > @@ -896,7 +894,6 @@ xrep_agi( > if (!xfs_sb_version_hasrmapbt(&mp->m_sb)) > return -EOPNOTSUPP; > > - xchk_perag_get(sc->mp, &sc->sa); > /* > * Make sure we have the AGI buffer, as scrub might have decided it > * was corrupt after xfs_ialloc_read_agi failed with -EFSCORRUPTED. > diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c > index 14e952d116f4..a5ca2856df8b 100644 > --- a/fs/xfs/scrub/bmap.c > +++ b/fs/xfs/scrub/bmap.c > @@ -260,7 +260,7 @@ xchk_bmap_iextent_xref( > agbno = XFS_FSB_TO_AGBNO(mp, irec->br_startblock); > len = irec->br_blockcount; > > - error = xchk_ag_init(info->sc, agno, &info->sc->sa); > + error = xchk_ag_init_existing(info->sc, agno, &info->sc->sa); > if (!xchk_fblock_process_error(info->sc, info->whichfork, > irec->br_startoff, &error)) > return; > diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c > index bd1172358964..c044e0a8da7f 100644 > --- a/fs/xfs/scrub/btree.c > +++ b/fs/xfs/scrub/btree.c > @@ -374,7 +374,7 @@ xchk_btree_check_block_owner( > > init_sa = bs->cur->bc_flags & XFS_BTREE_LONG_PTRS; > if (init_sa) { > - error = xchk_ag_init(bs->sc, agno, &bs->sc->sa); > + error = xchk_ag_init_existing(bs->sc, agno, &bs->sc->sa); > if (!xchk_btree_xref_process_error(bs->sc, bs->cur, > level, &error)) > return error; > diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c > index e86854171b0c..0ef96ed71017 100644 > --- a/fs/xfs/scrub/common.c > +++ b/fs/xfs/scrub/common.c > @@ -394,11 +394,11 @@ want_ag_read_header_failure( > } > > /* > - * Grab all the headers for an AG. > + * Grab the perag structure and all the headers for an AG. > * > - * The headers should be released by xchk_ag_free, but as a fail > - * safe we attach all the buffers we grab to the scrub transaction so > - * they'll all be freed when we cancel it. > + * The headers should be released by xchk_ag_free, but as a fail safe we attach > + * all the buffers we grab to the scrub transaction so they'll all be freed > + * when we cancel it. Returns ENOENT if we can't grab the perag structure. > */ > int > xchk_ag_read_headers( > @@ -409,22 +409,26 @@ xchk_ag_read_headers( > struct xfs_mount *mp = sc->mp; > int error; > > + ASSERT(!sa->pag); > + sa->pag = xfs_perag_get(mp, agno); > + if (!sa->pag) > + return -ENOENT; > + > sa->agno = agno; > > error = xfs_ialloc_read_agi(mp, sc->tp, agno, &sa->agi_bp); > if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGI)) > - goto out; > + return error; > > error = xfs_alloc_read_agf(mp, sc->tp, agno, 0, &sa->agf_bp); > if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGF)) > - goto out; > + return error; > > error = xfs_alloc_read_agfl(mp, sc->tp, agno, &sa->agfl_bp); > if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGFL)) > - goto out; > - error = 0; > -out: > - return error; > + return error; > + > + return 0; > } > > /* Release all the AG btree cursors. */ > @@ -461,7 +465,6 @@ xchk_ag_btcur_init( > { > struct xfs_mount *mp = sc->mp; > > - xchk_perag_get(sc->mp, sa); > if (sa->agf_bp && > xchk_ag_btree_healthy_enough(sc, sa->pag, XFS_BTNUM_BNO)) { > /* Set up a bnobt cursor for cross-referencing. */ > @@ -532,11 +535,11 @@ xchk_ag_free( > } > > /* > - * For scrub, grab the AGI and the AGF headers, in that order. Locking > - * order requires us to get the AGI before the AGF. We use the > - * transaction to avoid deadlocking on crosslinked metadata buffers; > - * either the caller passes one in (bmap scrub) or we have to create a > - * transaction ourselves. > + * For scrub, grab the perag structure, the AGI, and the AGF headers, in that > + * order. Locking order requires us to get the AGI before the AGF. We use the > + * transaction to avoid deadlocking on crosslinked metadata buffers; either the > + * caller passes one in (bmap scrub) or we have to create a transaction > + * ourselves. Returns ENOENT if the perag struct cannot be grabbed. > */ > int > xchk_ag_init( > @@ -554,19 +557,6 @@ xchk_ag_init( > return 0; > } > > -/* > - * Grab the per-ag structure if we haven't already gotten it. Teardown of the > - * xchk_ag will release it for us. > - */ > -void > -xchk_perag_get( > - struct xfs_mount *mp, > - struct xchk_ag *sa) > -{ > - if (!sa->pag) > - sa->pag = xfs_perag_get(mp, sa->agno); > -} > - > /* Per-scrubber setup functions */ > > /* > diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h > index 0410faf7d735..454145db10e7 100644 > --- a/fs/xfs/scrub/common.h > +++ b/fs/xfs/scrub/common.h > @@ -107,7 +107,23 @@ int xchk_setup_fscounters(struct xfs_scrub *sc); > void xchk_ag_free(struct xfs_scrub *sc, struct xchk_ag *sa); > int xchk_ag_init(struct xfs_scrub *sc, xfs_agnumber_t agno, > struct xchk_ag *sa); > -void xchk_perag_get(struct xfs_mount *mp, struct xchk_ag *sa); > + > +/* > + * Grab all AG resources, treating the inability to grab the perag structure as > + * a fs corruption. This is intended for callers checking an ondisk reference > + * to a given AG, which means that the AG must still exist. > + */ > +static inline int > +xchk_ag_init_existing( > + struct xfs_scrub *sc, > + xfs_agnumber_t agno, > + struct xchk_ag *sa) > +{ > + int error = xchk_ag_init(sc, agno, sa); > + > + return error == -ENOENT ? -EFSCORRUPTED : error; > +} > + > int xchk_ag_read_headers(struct xfs_scrub *sc, xfs_agnumber_t agno, > struct xchk_ag *sa); > void xchk_ag_btcur_free(struct xchk_ag *sa); > diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c > index e03577af63d9..74e331afe49d 100644 > --- a/fs/xfs/scrub/fscounters.c > +++ b/fs/xfs/scrub/fscounters.c > @@ -146,7 +146,7 @@ xchk_fscount_btreeblks( > xfs_extlen_t blocks; > int error; > > - error = xchk_ag_init(sc, agno, &sc->sa); > + error = xchk_ag_init_existing(sc, agno, &sc->sa); > if (error) > return error; > > diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c > index 76fbc7ca4cec..a6a68ba19f0a 100644 > --- a/fs/xfs/scrub/inode.c > +++ b/fs/xfs/scrub/inode.c > @@ -532,7 +532,7 @@ xchk_inode_xref( > agno = XFS_INO_TO_AGNO(sc->mp, ino); > agbno = XFS_INO_TO_AGBNO(sc->mp, ino); > > - error = xchk_ag_init(sc, agno, &sc->sa); > + error = xchk_ag_init_existing(sc, agno, &sc->sa); > if (!xchk_xref_process_error(sc, agno, agbno, &error)) > return; > -- chandan ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/5] xfs: dump log intent items that cannot be recovered due to corruption 2021-08-05 7:00 [PATCHSET 0/5] xfs: other stuff for 5.15 Darrick J. Wong ` (3 preceding siblings ...) 2021-08-05 7:01 ` [PATCH 4/5] xfs: grab active perag ref when reading AG headers Darrick J. Wong @ 2021-08-05 7:01 ` Darrick J. Wong 2021-08-06 11:41 ` Chandan Babu R 2021-08-09 14:55 ` Christoph Hellwig 4 siblings, 2 replies; 15+ messages in thread From: Darrick J. Wong @ 2021-08-05 7:01 UTC (permalink / raw) To: djwong; +Cc: linux-xfs From: Darrick J. Wong <djwong@kernel.org> If we try to recover a log intent item and the operation fails due to filesystem corruption, dump the contents of the item to the log for further analysis. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- fs/xfs/xfs_bmap_item.c | 3 +++ fs/xfs/xfs_extfree_item.c | 3 +++ fs/xfs/xfs_refcount_item.c | 3 +++ fs/xfs/xfs_rmap_item.c | 3 +++ 4 files changed, 12 insertions(+) diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c index e3a691937e92..3d6f70da8820 100644 --- a/fs/xfs/xfs_bmap_item.c +++ b/fs/xfs/xfs_bmap_item.c @@ -522,6 +522,9 @@ xfs_bui_item_recover( error = xfs_trans_log_finish_bmap_update(tp, budp, bui_type, ip, whichfork, bmap->me_startoff, bmap->me_startblock, &count, state); + if (error == -EFSCORRUPTED) + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bmap, + sizeof(*bmap)); if (error) goto err_cancel; diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c index 2424230ca2c3..3f8a0713573a 100644 --- a/fs/xfs/xfs_extfree_item.c +++ b/fs/xfs/xfs_extfree_item.c @@ -629,6 +629,9 @@ xfs_efi_item_recover( error = xfs_trans_free_extent(tp, efdp, extp->ext_start, extp->ext_len, &XFS_RMAP_OINFO_ANY_OWNER, false); + if (error == -EFSCORRUPTED) + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, + extp, sizeof(*extp)); if (error) goto abort_error; diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c index 746f4eda724c..163615285b18 100644 --- a/fs/xfs/xfs_refcount_item.c +++ b/fs/xfs/xfs_refcount_item.c @@ -522,6 +522,9 @@ xfs_cui_item_recover( error = xfs_trans_log_finish_refcount_update(tp, cudp, type, refc->pe_startblock, refc->pe_len, &new_fsb, &new_len, &rcur); + if (error == -EFSCORRUPTED) + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, + refc, sizeof(*refc)); if (error) goto abort_error; diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c index dc4f0c9f0897..9b91a788722a 100644 --- a/fs/xfs/xfs_rmap_item.c +++ b/fs/xfs/xfs_rmap_item.c @@ -578,6 +578,9 @@ xfs_rui_item_recover( rmap->me_owner, whichfork, rmap->me_startoff, rmap->me_startblock, rmap->me_len, state, &rcur); + if (error == -EFSCORRUPTED) + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, + rmap, sizeof(*rmap)); if (error) goto abort_error; ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] xfs: dump log intent items that cannot be recovered due to corruption 2021-08-05 7:01 ` [PATCH 5/5] xfs: dump log intent items that cannot be recovered due to corruption Darrick J. Wong @ 2021-08-06 11:41 ` Chandan Babu R 2021-08-09 14:55 ` Christoph Hellwig 1 sibling, 0 replies; 15+ messages in thread From: Chandan Babu R @ 2021-08-06 11:41 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On 05 Aug 2021 at 00:01, "Darrick J. Wong" <djwong@kernel.org> wrote: > From: Darrick J. Wong <djwong@kernel.org> > > If we try to recover a log intent item and the operation fails due to > filesystem corruption, dump the contents of the item to the log for > further analysis. > Looks good to me. Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/xfs_bmap_item.c | 3 +++ > fs/xfs/xfs_extfree_item.c | 3 +++ > fs/xfs/xfs_refcount_item.c | 3 +++ > fs/xfs/xfs_rmap_item.c | 3 +++ > 4 files changed, 12 insertions(+) > > > diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c > index e3a691937e92..3d6f70da8820 100644 > --- a/fs/xfs/xfs_bmap_item.c > +++ b/fs/xfs/xfs_bmap_item.c > @@ -522,6 +522,9 @@ xfs_bui_item_recover( > error = xfs_trans_log_finish_bmap_update(tp, budp, bui_type, ip, > whichfork, bmap->me_startoff, bmap->me_startblock, > &count, state); > + if (error == -EFSCORRUPTED) > + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bmap, > + sizeof(*bmap)); > if (error) > goto err_cancel; > > diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c > index 2424230ca2c3..3f8a0713573a 100644 > --- a/fs/xfs/xfs_extfree_item.c > +++ b/fs/xfs/xfs_extfree_item.c > @@ -629,6 +629,9 @@ xfs_efi_item_recover( > error = xfs_trans_free_extent(tp, efdp, extp->ext_start, > extp->ext_len, > &XFS_RMAP_OINFO_ANY_OWNER, false); > + if (error == -EFSCORRUPTED) > + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, > + extp, sizeof(*extp)); > if (error) > goto abort_error; > > diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c > index 746f4eda724c..163615285b18 100644 > --- a/fs/xfs/xfs_refcount_item.c > +++ b/fs/xfs/xfs_refcount_item.c > @@ -522,6 +522,9 @@ xfs_cui_item_recover( > error = xfs_trans_log_finish_refcount_update(tp, cudp, > type, refc->pe_startblock, refc->pe_len, > &new_fsb, &new_len, &rcur); > + if (error == -EFSCORRUPTED) > + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, > + refc, sizeof(*refc)); > if (error) > goto abort_error; > > diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c > index dc4f0c9f0897..9b91a788722a 100644 > --- a/fs/xfs/xfs_rmap_item.c > +++ b/fs/xfs/xfs_rmap_item.c > @@ -578,6 +578,9 @@ xfs_rui_item_recover( > rmap->me_owner, whichfork, > rmap->me_startoff, rmap->me_startblock, > rmap->me_len, state, &rcur); > + if (error == -EFSCORRUPTED) > + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, > + rmap, sizeof(*rmap)); > if (error) > goto abort_error; > -- chandan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] xfs: dump log intent items that cannot be recovered due to corruption 2021-08-05 7:01 ` [PATCH 5/5] xfs: dump log intent items that cannot be recovered due to corruption Darrick J. Wong 2021-08-06 11:41 ` Chandan Babu R @ 2021-08-09 14:55 ` Christoph Hellwig 1 sibling, 0 replies; 15+ messages in thread From: Christoph Hellwig @ 2021-08-09 14:55 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-08-09 15:08 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-05 7:00 [PATCHSET 0/5] xfs: other stuff for 5.15 Darrick J. Wong 2021-08-05 7:00 ` [PATCH 1/5] xfs: fix silly whitespace problems with kernel libxfs Darrick J. Wong 2021-08-06 5:45 ` Chandan Babu R 2021-08-09 14:51 ` Christoph Hellwig 2021-08-05 7:00 ` [PATCH 2/5] xfs: drop experimental warnings for bigtime and inobtcount Darrick J. Wong 2021-08-06 5:51 ` Chandan Babu R 2021-08-09 14:53 ` Christoph Hellwig 2021-08-05 7:01 ` [PATCH 3/5] xfs: automatic resource cleanup of for_each_perag* Darrick J. Wong 2021-08-06 7:15 ` Chandan Babu R 2021-08-09 15:07 ` Christoph Hellwig 2021-08-05 7:01 ` [PATCH 4/5] xfs: grab active perag ref when reading AG headers Darrick J. Wong 2021-08-06 11:25 ` Chandan Babu R 2021-08-05 7:01 ` [PATCH 5/5] xfs: dump log intent items that cannot be recovered due to corruption Darrick J. Wong 2021-08-06 11:41 ` Chandan Babu R 2021-08-09 14:55 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).