* [PATCH] repair: simplify bmap_next_offset
@ 2020-07-15 18:34 Christoph Hellwig
2020-07-15 23:55 ` Darrick J. Wong
0 siblings, 1 reply; 2+ messages in thread
From: Christoph Hellwig @ 2020-07-15 18:34 UTC (permalink / raw)
To: sandeen, linux-xfs
The tp argument is always NULL, and the whichfork argument is always
XFS_DATA_FORK, so simplify and cleanup the function based on those
assumptions.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
repair/phase6.c | 51 +++++++++++++++++++++++--------------------------
1 file changed, 24 insertions(+), 27 deletions(-)
diff --git a/repair/phase6.c b/repair/phase6.c
index 446bcfcb7..952af590f 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -406,46 +406,43 @@ dir_hash_dup_names(dir_hash_tab_t *hashtab)
}
/*
- * Given a block number in a fork, return the next valid block number
- * (not a hole).
- * If this is the last block number then NULLFILEOFF is returned.
- *
- * This was originally in the kernel, but only used in xfs_repair.
+ * Given a block number in a fork, return the next valid block number (not a
+ * hole). If this is the last block number then NULLFILEOFF is returned.
*/
static int
bmap_next_offset(
- xfs_trans_t *tp, /* transaction pointer */
- xfs_inode_t *ip, /* incore inode */
- xfs_fileoff_t *bnop, /* current block */
- int whichfork) /* data or attr fork */
+ struct xfs_inode *ip,
+ xfs_fileoff_t *bnop)
{
- xfs_fileoff_t bno; /* current block */
- int error; /* error return value */
- xfs_bmbt_irec_t got; /* current extent value */
- struct xfs_ifork *ifp; /* inode fork pointer */
+ xfs_fileoff_t bno;
+ int error;
+ struct xfs_bmbt_irec got;
struct xfs_iext_cursor icur;
- if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
- return EIO;
- if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
+ switch (ip->i_d.di_format) {
+ case XFS_DINODE_FMT_LOCAL:
*bnop = NULLFILEOFF;
return 0;
+ case XFS_DINODE_FMT_BTREE:
+ case XFS_DINODE_FMT_EXTENTS:
+ break;
+ default:
+ return EIO;
+ }
+
+ if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
+ error = -libxfs_iread_extents(NULL, ip, XFS_DATA_FORK);
+ if (error)
+ return error;
}
- ifp = XFS_IFORK_PTR(ip, whichfork);
- if (!(ifp->if_flags & XFS_IFEXTENTS) &&
- (error = -libxfs_iread_extents(tp, ip, whichfork)))
- return error;
bno = *bnop + 1;
- if (!libxfs_iext_lookup_extent(ip, ifp, bno, &icur, &got))
+ if (!libxfs_iext_lookup_extent(ip, &ip->i_df, bno, &icur, &got))
*bnop = NULLFILEOFF;
else
*bnop = got.br_startoff < bno ? bno : got.br_startoff;
return 0;
}
-
static void
res_failed(
int err)
@@ -2054,7 +2051,7 @@ longform_dir2_check_node(
next_da_bno != NULLFILEOFF && da_bno < mp->m_dir_geo->freeblk;
da_bno = (xfs_dablk_t)next_da_bno) {
next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
- if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK))
+ if (bmap_next_offset(ip, &next_da_bno))
break;
/*
@@ -2129,7 +2126,7 @@ longform_dir2_check_node(
next_da_bno != NULLFILEOFF;
da_bno = (xfs_dablk_t)next_da_bno) {
next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
- if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK))
+ if (bmap_next_offset(ip, &next_da_bno))
break;
error = dir_read_buf(ip, da_bno, &bp, &xfs_dir3_free_buf_ops,
@@ -2261,7 +2258,7 @@ longform_dir2_entry_check(xfs_mount_t *mp,
struct xfs_dir2_data_hdr *d;
next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
- if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK)) {
+ if (bmap_next_offset(ip, &next_da_bno)) {
/*
* if this is the first block, there isn't anything we
* can recover so we just trash it.
--
2.27.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] repair: simplify bmap_next_offset
2020-07-15 18:34 [PATCH] repair: simplify bmap_next_offset Christoph Hellwig
@ 2020-07-15 23:55 ` Darrick J. Wong
0 siblings, 0 replies; 2+ messages in thread
From: Darrick J. Wong @ 2020-07-15 23:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sandeen, linux-xfs
On Wed, Jul 15, 2020 at 08:34:17PM +0200, Christoph Hellwig wrote:
> The tp argument is always NULL, and the whichfork argument is always
> XFS_DATA_FORK, so simplify and cleanup the function based on those
> assumptions.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
/me has nearly the same code change in his 5.8 sync branch.
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> repair/phase6.c | 51 +++++++++++++++++++++++--------------------------
> 1 file changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 446bcfcb7..952af590f 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -406,46 +406,43 @@ dir_hash_dup_names(dir_hash_tab_t *hashtab)
> }
>
> /*
> - * Given a block number in a fork, return the next valid block number
> - * (not a hole).
> - * If this is the last block number then NULLFILEOFF is returned.
> - *
> - * This was originally in the kernel, but only used in xfs_repair.
> + * Given a block number in a fork, return the next valid block number (not a
> + * hole). If this is the last block number then NULLFILEOFF is returned.
> */
> static int
> bmap_next_offset(
> - xfs_trans_t *tp, /* transaction pointer */
> - xfs_inode_t *ip, /* incore inode */
> - xfs_fileoff_t *bnop, /* current block */
> - int whichfork) /* data or attr fork */
> + struct xfs_inode *ip,
> + xfs_fileoff_t *bnop)
> {
> - xfs_fileoff_t bno; /* current block */
> - int error; /* error return value */
> - xfs_bmbt_irec_t got; /* current extent value */
> - struct xfs_ifork *ifp; /* inode fork pointer */
> + xfs_fileoff_t bno;
> + int error;
> + struct xfs_bmbt_irec got;
> struct xfs_iext_cursor icur;
>
> - if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
> - XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
> - XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL)
> - return EIO;
> - if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
> + switch (ip->i_d.di_format) {
> + case XFS_DINODE_FMT_LOCAL:
> *bnop = NULLFILEOFF;
> return 0;
> + case XFS_DINODE_FMT_BTREE:
> + case XFS_DINODE_FMT_EXTENTS:
> + break;
> + default:
> + return EIO;
> + }
> +
> + if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
> + error = -libxfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> + if (error)
> + return error;
> }
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> - if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> - (error = -libxfs_iread_extents(tp, ip, whichfork)))
> - return error;
> bno = *bnop + 1;
> - if (!libxfs_iext_lookup_extent(ip, ifp, bno, &icur, &got))
> + if (!libxfs_iext_lookup_extent(ip, &ip->i_df, bno, &icur, &got))
> *bnop = NULLFILEOFF;
> else
> *bnop = got.br_startoff < bno ? bno : got.br_startoff;
> return 0;
> }
>
> -
> static void
> res_failed(
> int err)
> @@ -2054,7 +2051,7 @@ longform_dir2_check_node(
> next_da_bno != NULLFILEOFF && da_bno < mp->m_dir_geo->freeblk;
> da_bno = (xfs_dablk_t)next_da_bno) {
> next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
> - if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK))
> + if (bmap_next_offset(ip, &next_da_bno))
> break;
>
> /*
> @@ -2129,7 +2126,7 @@ longform_dir2_check_node(
> next_da_bno != NULLFILEOFF;
> da_bno = (xfs_dablk_t)next_da_bno) {
> next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
> - if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK))
> + if (bmap_next_offset(ip, &next_da_bno))
> break;
>
> error = dir_read_buf(ip, da_bno, &bp, &xfs_dir3_free_buf_ops,
> @@ -2261,7 +2258,7 @@ longform_dir2_entry_check(xfs_mount_t *mp,
> struct xfs_dir2_data_hdr *d;
>
> next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
> - if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK)) {
> + if (bmap_next_offset(ip, &next_da_bno)) {
> /*
> * if this is the first block, there isn't anything we
> * can recover so we just trash it.
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-07-15 23:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 18:34 [PATCH] repair: simplify bmap_next_offset Christoph Hellwig
2020-07-15 23:55 ` Darrick J. Wong
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).