[PATCH 0/5] xfs: quota verifier/repair improvements take 2

[PATCH 0/5] xfs: quota verifier/repair improvements take 2

[Eric Sandeen]

A new attempt at the quota improvements regarding in-kernel repair 
and verifier behavior.

This series structures things a bit differently to avoid the log
replay verifier failure Darrick hit :( and also uses his new
sub-buffer verifier error function to print the specific corrupted
quota.

Ran this one through -g quick,auto with "-o quota" and saw no
regressions.

For now I've left out the last 2 patches from last time which try to
quiet down the kernel messages when mounting/quotachecking a corrupted
fs, I want to think about those & let them soak a bit more.

Thanks,
-Eric

[PATCH 1/5] xfs: remove unused flags arg from xfs_dquot_verify

[Eric Sandeen]

Long ago the flags argument was used to determine whether to issue warnings
about corruptions, but that's done elsewhere now and the flag is unused
here, so remove it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c  | 5 ++---
 fs/xfs/libxfs/xfs_quota_defs.h | 3 +--
 fs/xfs/xfs_dquot.c             | 2 +-
 fs/xfs/xfs_log_recover.c       | 4 ++--
 fs/xfs/xfs_qm.c                | 2 +-
 5 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 8b7a6c3cb599..a926058f7b0c 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -47,8 +47,7 @@ xfs_dquot_verify(
 	struct xfs_mount *mp,
 	xfs_disk_dquot_t *ddq,
 	xfs_dqid_t	 id,
-	uint		 type,	  /* used only when IO_dorepair is true */
-	uint		 flags)
+	uint		 type)	  /* used only when IO_dorepair is true */
 {
 	/*
 	 * We can encounter an uninitialized dquot buffer for 2 reasons:
@@ -200,7 +199,7 @@ xfs_dquot_buf_verify(
 		if (i == 0)
 			id = be32_to_cpu(ddq->d_id);
 
-		fa = xfs_dquot_verify(mp, ddq, id + i, 0, 0);
+		fa = xfs_dquot_verify(mp, ddq, id + i, 0);
 		if (fa)
 			return fa;
 	}
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index bb1b13a9b5f4..8433656af0da 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -152,8 +152,7 @@ typedef uint16_t	xfs_qwarncnt_t;
 #define XFS_QMOPT_RESBLK_MASK	(XFS_QMOPT_RES_REGBLKS | XFS_QMOPT_RES_RTBLKS)
 
 extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
-		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type,
-		uint flags);
+		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type);
 extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
 extern int xfs_dquot_repair(struct xfs_mount *mp, struct xfs_disk_dquot *ddq,
 		xfs_dqid_t id, uint type);
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index a7daef9e16bf..8d378f485260 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -1001,7 +1001,7 @@ xfs_qm_dqflush(
 	/*
 	 * A simple sanity check in case we got a corrupted dquot..
 	 */
-	fa = xfs_dquot_verify(mp, &dqp->q_core, be32_to_cpu(ddqp->d_id), 0, 0);
+	fa = xfs_dquot_verify(mp, &dqp->q_core, be32_to_cpu(ddqp->d_id), 0);
 	if (fa) {
 		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
 				be32_to_cpu(ddqp->d_id), fa);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 2b2383f1895e..06a09cb948b5 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2702,7 +2702,7 @@ xlog_recover_do_reg_buffer(
 				goto next;
 			}
 			fa = xfs_dquot_verify(mp, item->ri_buf[i].i_addr,
-					       -1, 0, 0);
+					       -1, 0);
 			if (fa) {
 				xfs_alert(mp,
 	"dquot corrupt at %pS trying to replay into block 0x%llx",
@@ -3348,7 +3348,7 @@ xlog_recover_dquot_pass2(
 	 */
 	dq_f = item->ri_buf[0].i_addr;
 	ASSERT(dq_f);
-	fa = xfs_dquot_verify(mp, recddq, dq_f->qlf_id, 0, 0);
+	fa = xfs_dquot_verify(mp, recddq, dq_f->qlf_id, 0);
 	if (fa) {
 		xfs_alert(mp, "corrupt dquot ID 0x%x in log at %pS",
 				dq_f->qlf_id, fa);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index ec39ae274c78..c71ad79bb2e6 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -865,7 +865,7 @@ xfs_qm_reset_dqcounts(
 		 * find uninitialised dquot blks. See comment in
 		 * xfs_dquot_verify.
 		 */
-		fa = xfs_dquot_verify(mp, ddq, id + j, type, 0);
+		fa = xfs_dquot_verify(mp, ddq, id + j, type);
 		if (fa)
 			xfs_dquot_repair(mp, ddq, id + j, type);
 
-- 
2.17.0

[PATCH 2/5] xfs: check type in quota verifier during quotacheck

[Eric Sandeen]

During quotacheck we send in the quota type, so verify that as well.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index a926058f7b0c..c00174f32f0d 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -47,7 +47,7 @@ xfs_dquot_verify(
 	struct xfs_mount *mp,
 	xfs_disk_dquot_t *ddq,
 	xfs_dqid_t	 id,
-	uint		 type)	  /* used only when IO_dorepair is true */
+	uint		 type)	  /* used only during quotacheck */
 {
 	/*
 	 * We can encounter an uninitialized dquot buffer for 2 reasons:
@@ -69,6 +69,8 @@ xfs_dquot_verify(
 	if (ddq->d_version != XFS_DQUOT_VERSION)
 		return __this_address;
 
+	if (type && ddq->d_flags != type)
+		return __this_address;
 	if (ddq->d_flags != XFS_DQ_USER &&
 	    ddq->d_flags != XFS_DQ_PROJ &&
 	    ddq->d_flags != XFS_DQ_GROUP)
-- 
2.17.0

Re: [PATCH 2/5] xfs: check type in quota verifier during quotacheck

[Darrick J. Wong]

On Fri, May 04, 2018 at 12:34:22PM -0500, Eric Sandeen wrote:
> During quotacheck we send in the quota type, so verify that as well.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_dquot_buf.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index a926058f7b0c..c00174f32f0d 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -47,7 +47,7 @@ xfs_dquot_verify(
>  	struct xfs_mount *mp,
>  	xfs_disk_dquot_t *ddq,
>  	xfs_dqid_t	 id,
> -	uint		 type)	  /* used only when IO_dorepair is true */
> +	uint		 type)	  /* used only during quotacheck */
>  {
>  	/*
>  	 * We can encounter an uninitialized dquot buffer for 2 reasons:
> @@ -69,6 +69,8 @@ xfs_dquot_verify(
>  	if (ddq->d_version != XFS_DQUOT_VERSION)
>  		return __this_address;
>  
> +	if (type && ddq->d_flags != type)
> +		return __this_address;
>  	if (ddq->d_flags != XFS_DQ_USER &&
>  	    ddq->d_flags != XFS_DQ_PROJ &&
>  	    ddq->d_flags != XFS_DQ_GROUP)
> -- 
> 2.17.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] xfs: check type in quota verifier during quotacheck

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/5] xfs: repair full xfs_dqblk during quotacheck

[Eric Sandeen]

It's a bit dicey to pass in the smaller xfs_disk_dquot and then cast it to
something larger; pass in the full xfs_dqblk so we know the caller has sent
us the right thing.  Rename the function to xfs_dqblk_repair for
clarity.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c  | 21 +++++++++------------
 fs/xfs/libxfs/xfs_quota_defs.h |  2 +-
 fs/xfs/xfs_qm.c                |  2 +-
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index c00174f32f0d..3b92427883fa 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -104,29 +104,26 @@ xfs_dquot_verify(
  * Do some primitive error checking on ondisk dquot data structures.
  */
 int
-xfs_dquot_repair(
+xfs_dqblk_repair(
 	struct xfs_mount	*mp,
-	struct xfs_disk_dquot	*ddq,
+	struct xfs_dqblk	*dqb,
 	xfs_dqid_t		id,
 	uint			type)
 {
-	struct xfs_dqblk	*d = (struct xfs_dqblk *)ddq;
-
-
 	/*
 	 * Typically, a repair is only requested by quotacheck.
 	 */
 	ASSERT(id != -1);
-	memset(d, 0, sizeof(xfs_dqblk_t));
+	memset(dqb, 0, sizeof(xfs_dqblk_t));
 
-	d->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
-	d->dd_diskdq.d_version = XFS_DQUOT_VERSION;
-	d->dd_diskdq.d_flags = type;
-	d->dd_diskdq.d_id = cpu_to_be32(id);
+	dqb->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
+	dqb->dd_diskdq.d_version = XFS_DQUOT_VERSION;
+	dqb->dd_diskdq.d_flags = type;
+	dqb->dd_diskdq.d_id = cpu_to_be32(id);
 
 	if (xfs_sb_version_hascrc(&mp->m_sb)) {
-		uuid_copy(&d->dd_uuid, &mp->m_sb.sb_meta_uuid);
-		xfs_update_cksum((char *)d, sizeof(struct xfs_dqblk),
+		uuid_copy(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid);
+		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
 	}
 
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index 8433656af0da..a2f8cb334bb3 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -154,7 +154,7 @@ typedef uint16_t	xfs_qwarncnt_t;
 extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
 		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type);
 extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
-extern int xfs_dquot_repair(struct xfs_mount *mp, struct xfs_disk_dquot *ddq,
+extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
 		xfs_dqid_t id, uint type);
 
 #endif	/* __XFS_QUOTA_H__ */
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index c71ad79bb2e6..6690b938d450 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -867,7 +867,7 @@ xfs_qm_reset_dqcounts(
 		 */
 		fa = xfs_dquot_verify(mp, ddq, id + j, type);
 		if (fa)
-			xfs_dquot_repair(mp, ddq, id + j, type);
+			xfs_dqblk_repair(mp, &dqb[j], id + j, type);
 
 		/*
 		 * Reset type in case we are reusing group quota file for
-- 
2.17.0

Re: [PATCH 3/5] xfs: repair full xfs_dqblk during quotacheck

[Darrick J. Wong]

On Fri, May 04, 2018 at 12:34:49PM -0500, Eric Sandeen wrote:
> It's a bit dicey to pass in the smaller xfs_disk_dquot and then cast it to
> something larger; pass in the full xfs_dqblk so we know the caller has sent
> us the right thing.  Rename the function to xfs_dqblk_repair for
> clarity.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_dquot_buf.c  | 21 +++++++++------------
>  fs/xfs/libxfs/xfs_quota_defs.h |  2 +-
>  fs/xfs/xfs_qm.c                |  2 +-
>  3 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index c00174f32f0d..3b92427883fa 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -104,29 +104,26 @@ xfs_dquot_verify(
>   * Do some primitive error checking on ondisk dquot data structures.
>   */
>  int
> -xfs_dquot_repair(
> +xfs_dqblk_repair(
>  	struct xfs_mount	*mp,
> -	struct xfs_disk_dquot	*ddq,
> +	struct xfs_dqblk	*dqb,
>  	xfs_dqid_t		id,
>  	uint			type)
>  {
> -	struct xfs_dqblk	*d = (struct xfs_dqblk *)ddq;
> -
> -
>  	/*
>  	 * Typically, a repair is only requested by quotacheck.
>  	 */
>  	ASSERT(id != -1);
> -	memset(d, 0, sizeof(xfs_dqblk_t));
> +	memset(dqb, 0, sizeof(xfs_dqblk_t));
>  
> -	d->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> -	d->dd_diskdq.d_version = XFS_DQUOT_VERSION;
> -	d->dd_diskdq.d_flags = type;
> -	d->dd_diskdq.d_id = cpu_to_be32(id);
> +	dqb->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> +	dqb->dd_diskdq.d_version = XFS_DQUOT_VERSION;
> +	dqb->dd_diskdq.d_flags = type;
> +	dqb->dd_diskdq.d_id = cpu_to_be32(id);
>  
>  	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> -		uuid_copy(&d->dd_uuid, &mp->m_sb.sb_meta_uuid);
> -		xfs_update_cksum((char *)d, sizeof(struct xfs_dqblk),
> +		uuid_copy(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid);
> +		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
>  				 XFS_DQUOT_CRC_OFF);
>  	}
>  
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index 8433656af0da..a2f8cb334bb3 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -154,7 +154,7 @@ typedef uint16_t	xfs_qwarncnt_t;
>  extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
>  		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type);
>  extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
> -extern int xfs_dquot_repair(struct xfs_mount *mp, struct xfs_disk_dquot *ddq,
> +extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
>  		xfs_dqid_t id, uint type);
>  
>  #endif	/* __XFS_QUOTA_H__ */
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index c71ad79bb2e6..6690b938d450 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -867,7 +867,7 @@ xfs_qm_reset_dqcounts(
>  		 */
>  		fa = xfs_dquot_verify(mp, ddq, id + j, type);
>  		if (fa)
> -			xfs_dquot_repair(mp, ddq, id + j, type);
> +			xfs_dqblk_repair(mp, &dqb[j], id + j, type);
>  
>  		/*
>  		 * Reset type in case we are reusing group quota file for
> -- 
> 2.17.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] xfs: repair full xfs_dqblk during quotacheck

[Christoph Hellwig]

I think this commit is mislabeld, it onl passes the xfs_dqblk,
but doesn't actually change any functionality as far as I can tell.

>  		if (fa)
> -			xfs_dquot_repair(mp, ddq, id + j, type);
> +			xfs_dqblk_repair(mp, &dqb[j], id + j, type);

Also please pick one of the two possible array index / pointer
arithmetics notations and stick to them in a single statement at least.

[PATCH 3/5 V2] xfs: pass full xfs_dqblk to repair during quotacheck

[Eric Sandeen]

It's a bit dicey to pass in the smaller xfs_disk_dquot and then cast it to
something larger; pass in the full xfs_dqblk so we know the caller has sent
us the right thing.  Rename the function to xfs_dqblk_repair for
clarity.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: More precise commit summary

 fs/xfs/libxfs/xfs_dquot_buf.c  | 21 +++++++++------------
 fs/xfs/libxfs/xfs_quota_defs.h |  2 +-
 fs/xfs/xfs_qm.c                |  2 +-
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index c00174f32f0d..3b92427883fa 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -104,29 +104,26 @@ xfs_dquot_verify(
  * Do some primitive error checking on ondisk dquot data structures.
  */
 int
-xfs_dquot_repair(
+xfs_dqblk_repair(
 	struct xfs_mount	*mp,
-	struct xfs_disk_dquot	*ddq,
+	struct xfs_dqblk	*dqb,
 	xfs_dqid_t		id,
 	uint			type)
 {
-	struct xfs_dqblk	*d = (struct xfs_dqblk *)ddq;
-
-
 	/*
 	 * Typically, a repair is only requested by quotacheck.
 	 */
 	ASSERT(id != -1);
-	memset(d, 0, sizeof(xfs_dqblk_t));
+	memset(dqb, 0, sizeof(xfs_dqblk_t));
 
-	d->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
-	d->dd_diskdq.d_version = XFS_DQUOT_VERSION;
-	d->dd_diskdq.d_flags = type;
-	d->dd_diskdq.d_id = cpu_to_be32(id);
+	dqb->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
+	dqb->dd_diskdq.d_version = XFS_DQUOT_VERSION;
+	dqb->dd_diskdq.d_flags = type;
+	dqb->dd_diskdq.d_id = cpu_to_be32(id);
 
 	if (xfs_sb_version_hascrc(&mp->m_sb)) {
-		uuid_copy(&d->dd_uuid, &mp->m_sb.sb_meta_uuid);
-		xfs_update_cksum((char *)d, sizeof(struct xfs_dqblk),
+		uuid_copy(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid);
+		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
 	}
 
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index 8433656af0da..a2f8cb334bb3 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -154,7 +154,7 @@ typedef uint16_t	xfs_qwarncnt_t;
 extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
 		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type);
 extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
-extern int xfs_dquot_repair(struct xfs_mount *mp, struct xfs_disk_dquot *ddq,
+extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
 		xfs_dqid_t id, uint type);
 
 #endif	/* __XFS_QUOTA_H__ */
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index c71ad79bb2e6..6690b938d450 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -867,7 +867,7 @@ xfs_qm_reset_dqcounts(
 		 */
 		fa = xfs_dquot_verify(mp, ddq, id + j, type);
 		if (fa)
-			xfs_dquot_repair(mp, ddq, id + j, type);
+			xfs_dqblk_repair(mp, &dqb[j], id + j, type);
 
 		/*
 		 * Reset type in case we are reusing group quota file for
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5 V2] xfs: pass full xfs_dqblk to repair during quotacheck

[Darrick J. Wong]

On Mon, May 07, 2018 at 10:22:05AM -0500, Eric Sandeen wrote:
> It's a bit dicey to pass in the smaller xfs_disk_dquot and then cast it to
> something larger; pass in the full xfs_dqblk so we know the caller has sent
> us the right thing.  Rename the function to xfs_dqblk_repair for
> clarity.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> V2: More precise commit summary
> 
>  fs/xfs/libxfs/xfs_dquot_buf.c  | 21 +++++++++------------
>  fs/xfs/libxfs/xfs_quota_defs.h |  2 +-
>  fs/xfs/xfs_qm.c                |  2 +-
>  3 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index c00174f32f0d..3b92427883fa 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -104,29 +104,26 @@ xfs_dquot_verify(
>   * Do some primitive error checking on ondisk dquot data structures.
>   */
>  int
> -xfs_dquot_repair(
> +xfs_dqblk_repair(
>  	struct xfs_mount	*mp,
> -	struct xfs_disk_dquot	*ddq,
> +	struct xfs_dqblk	*dqb,
>  	xfs_dqid_t		id,
>  	uint			type)
>  {
> -	struct xfs_dqblk	*d = (struct xfs_dqblk *)ddq;
> -
> -
>  	/*
>  	 * Typically, a repair is only requested by quotacheck.
>  	 */
>  	ASSERT(id != -1);
> -	memset(d, 0, sizeof(xfs_dqblk_t));
> +	memset(dqb, 0, sizeof(xfs_dqblk_t));
>  
> -	d->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> -	d->dd_diskdq.d_version = XFS_DQUOT_VERSION;
> -	d->dd_diskdq.d_flags = type;
> -	d->dd_diskdq.d_id = cpu_to_be32(id);
> +	dqb->dd_diskdq.d_magic = cpu_to_be16(XFS_DQUOT_MAGIC);
> +	dqb->dd_diskdq.d_version = XFS_DQUOT_VERSION;
> +	dqb->dd_diskdq.d_flags = type;
> +	dqb->dd_diskdq.d_id = cpu_to_be32(id);
>  
>  	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> -		uuid_copy(&d->dd_uuid, &mp->m_sb.sb_meta_uuid);
> -		xfs_update_cksum((char *)d, sizeof(struct xfs_dqblk),
> +		uuid_copy(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid);
> +		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
>  				 XFS_DQUOT_CRC_OFF);
>  	}
>  
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index 8433656af0da..a2f8cb334bb3 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -154,7 +154,7 @@ typedef uint16_t	xfs_qwarncnt_t;
>  extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
>  		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type);
>  extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
> -extern int xfs_dquot_repair(struct xfs_mount *mp, struct xfs_disk_dquot *ddq,
> +extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
>  		xfs_dqid_t id, uint type);
>  
>  #endif	/* __XFS_QUOTA_H__ */
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index c71ad79bb2e6..6690b938d450 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -867,7 +867,7 @@ xfs_qm_reset_dqcounts(
>  		 */
>  		fa = xfs_dquot_verify(mp, ddq, id + j, type);
>  		if (fa)
> -			xfs_dquot_repair(mp, ddq, id + j, type);
> +			xfs_dqblk_repair(mp, &dqb[j], id + j, type);
>  
>  		/*
>  		 * Reset type in case we are reusing group quota file for
> -- 
> 2.17.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] xfs: add full xfs_dqblk verifier

[Eric Sandeen]

Add an xfs_dqblk verifier so that it can check the uuid on V5 filesystems;
it calls the existing xfs_dquot_verify verifier to validate the
xfs_disk_dquot_t contained inside it.  This lets us move the uuid
verification out of the crc verifier, which makes little sense.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c  | 27 ++++++++++++++++++++++-----
 fs/xfs/libxfs/xfs_quota_defs.h |  2 ++
 fs/xfs/xfs_dquot.c             | 10 +++++-----
 fs/xfs/xfs_qm.c                |  2 +-
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 3b92427883fa..d71c0b53536b 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -41,7 +41,12 @@ xfs_calc_dquots_per_chunk(
 
 /*
  * Do some primitive error checking on ondisk dquot data structures.
+ *
+ * The xfs_dqblk structure /contains/ the xfs_disk_dquot structure;
+ * we verify them separately because at some points we have only the
+ * smaller xfs_disk_dquot structure available.
  */
+
 xfs_failaddr_t
 xfs_dquot_verify(
 	struct xfs_mount *mp,
@@ -100,6 +105,20 @@ xfs_dquot_verify(
 	return NULL;
 }
 
+xfs_failaddr_t
+xfs_dqblk_verify(
+	struct xfs_mount	*mp,
+	struct xfs_dqblk	*dqb,
+	xfs_dqid_t	 	id,
+	uint		 	type)	/* used only during quotacheck */
+{
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	    !uuid_equal(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid))
+			return __this_address;
+
+	return xfs_dquot_verify(mp, &dqb->dd_diskdq, id, type);
+}
+
 /*
  * Do some primitive error checking on ondisk dquot data structures.
  */
@@ -156,8 +175,6 @@ xfs_dquot_buf_verify_crc(
 		if (!xfs_verify_cksum((char *)d, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF))
 			return false;
-		if (!uuid_equal(&d->dd_uuid, &mp->m_sb.sb_meta_uuid))
-			return false;
 	}
 	return true;
 }
@@ -167,7 +184,7 @@ xfs_dquot_buf_verify(
 	struct xfs_mount	*mp,
 	struct xfs_buf		*bp)
 {
-	struct xfs_dqblk	*d = (struct xfs_dqblk *)bp->b_addr;
+	struct xfs_dqblk	*dqb = (struct xfs_dqblk *)bp->b_addr;
 	xfs_failaddr_t		fa;
 	xfs_dqid_t		id = 0;
 	int			ndquots;
@@ -193,12 +210,12 @@ xfs_dquot_buf_verify(
 	for (i = 0; i < ndquots; i++) {
 		struct xfs_disk_dquot	*ddq;
 
-		ddq = &d[i].dd_diskdq;
+		ddq = &dqb[i].dd_diskdq;
 
 		if (i == 0)
 			id = be32_to_cpu(ddq->d_id);
 
-		fa = xfs_dquot_verify(mp, ddq, id + i, 0);
+		fa = xfs_dqblk_verify(mp, &dqb[i], id + i, 0);
 		if (fa)
 			return fa;
 	}
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index a2f8cb334bb3..1aac52d7fef4 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -153,6 +153,8 @@ typedef uint16_t	xfs_qwarncnt_t;
 
 extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
 		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type);
+extern xfs_failaddr_t xfs_dqblk_verify(struct xfs_mount *mp,
+		struct xfs_dqblk *dqb, xfs_dqid_t id, uint type);
 extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
 extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
 		xfs_dqid_t id, uint type);
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 8d378f485260..d0880c1add41 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -953,6 +953,7 @@ xfs_qm_dqflush(
 {
 	struct xfs_mount	*mp = dqp->q_mount;
 	struct xfs_buf		*bp;
+	struct xfs_dqblk	*dqb;
 	struct xfs_disk_dquot	*ddqp;
 	xfs_failaddr_t		fa;
 	int			error;
@@ -996,12 +997,13 @@ xfs_qm_dqflush(
 	/*
 	 * Calculate the location of the dquot inside the buffer.
 	 */
-	ddqp = bp->b_addr + dqp->q_bufoffset;
+	dqb = bp->b_addr + dqp->q_bufoffset;
+	ddqp = &dqb->dd_diskdq;
 
 	/*
-	 * A simple sanity check in case we got a corrupted dquot..
+	 * A simple sanity check in case we got a corrupted dquot.
 	 */
-	fa = xfs_dquot_verify(mp, &dqp->q_core, be32_to_cpu(ddqp->d_id), 0);
+	fa = xfs_dqblk_verify(mp, dqb, be32_to_cpu(ddqp->d_id), 0);
 	if (fa) {
 		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
 				be32_to_cpu(ddqp->d_id), fa);
@@ -1032,8 +1034,6 @@ xfs_qm_dqflush(
 	 * of a dquot without an up-to-date CRC getting to disk.
 	 */
 	if (xfs_sb_version_hascrc(&mp->m_sb)) {
-		struct xfs_dqblk *dqb = (struct xfs_dqblk *)ddqp;
-
 		dqb->dd_lsn = cpu_to_be64(dqp->q_logitem.qli_item.li_lsn);
 		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 6690b938d450..3138c06b68a6 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -865,7 +865,7 @@ xfs_qm_reset_dqcounts(
 		 * find uninitialised dquot blks. See comment in
 		 * xfs_dquot_verify.
 		 */
-		fa = xfs_dquot_verify(mp, ddq, id + j, type);
+		fa = xfs_dqblk_verify(mp, &dqb[j], id + j, type);
 		if (fa)
 			xfs_dqblk_repair(mp, &dqb[j], id + j, type);
 
-- 
2.17.0

Re: [PATCH 4/5] xfs: add full xfs_dqblk verifier

[Darrick J. Wong]

On Fri, May 04, 2018 at 12:35:05PM -0500, Eric Sandeen wrote:
> Add an xfs_dqblk verifier so that it can check the uuid on V5 filesystems;
> it calls the existing xfs_dquot_verify verifier to validate the
> xfs_disk_dquot_t contained inside it.  This lets us move the uuid
> verification out of the crc verifier, which makes little sense.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_dquot_buf.c  | 27 ++++++++++++++++++++++-----
>  fs/xfs/libxfs/xfs_quota_defs.h |  2 ++
>  fs/xfs/xfs_dquot.c             | 10 +++++-----
>  fs/xfs/xfs_qm.c                |  2 +-
>  4 files changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index 3b92427883fa..d71c0b53536b 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -41,7 +41,12 @@ xfs_calc_dquots_per_chunk(
>  
>  /*
>   * Do some primitive error checking on ondisk dquot data structures.
> + *
> + * The xfs_dqblk structure /contains/ the xfs_disk_dquot structure;
> + * we verify them separately because at some points we have only the
> + * smaller xfs_disk_dquot structure available.
>   */
> +
>  xfs_failaddr_t
>  xfs_dquot_verify(
>  	struct xfs_mount *mp,
> @@ -100,6 +105,20 @@ xfs_dquot_verify(
>  	return NULL;
>  }
>  
> +xfs_failaddr_t
> +xfs_dqblk_verify(
> +	struct xfs_mount	*mp,
> +	struct xfs_dqblk	*dqb,
> +	xfs_dqid_t	 	id,
> +	uint		 	type)	/* used only during quotacheck */
> +{
> +	if (xfs_sb_version_hascrc(&mp->m_sb) &&
> +	    !uuid_equal(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid))
> +			return __this_address;
> +
> +	return xfs_dquot_verify(mp, &dqb->dd_diskdq, id, type);
> +}
> +
>  /*
>   * Do some primitive error checking on ondisk dquot data structures.
>   */
> @@ -156,8 +175,6 @@ xfs_dquot_buf_verify_crc(
>  		if (!xfs_verify_cksum((char *)d, sizeof(struct xfs_dqblk),
>  				 XFS_DQUOT_CRC_OFF))
>  			return false;
> -		if (!uuid_equal(&d->dd_uuid, &mp->m_sb.sb_meta_uuid))
> -			return false;
>  	}
>  	return true;
>  }
> @@ -167,7 +184,7 @@ xfs_dquot_buf_verify(
>  	struct xfs_mount	*mp,
>  	struct xfs_buf		*bp)
>  {
> -	struct xfs_dqblk	*d = (struct xfs_dqblk *)bp->b_addr;
> +	struct xfs_dqblk	*dqb = (struct xfs_dqblk *)bp->b_addr;
>  	xfs_failaddr_t		fa;
>  	xfs_dqid_t		id = 0;
>  	int			ndquots;
> @@ -193,12 +210,12 @@ xfs_dquot_buf_verify(
>  	for (i = 0; i < ndquots; i++) {
>  		struct xfs_disk_dquot	*ddq;
>  
> -		ddq = &d[i].dd_diskdq;
> +		ddq = &dqb[i].dd_diskdq;
>  
>  		if (i == 0)
>  			id = be32_to_cpu(ddq->d_id);
>  
> -		fa = xfs_dquot_verify(mp, ddq, id + i, 0);
> +		fa = xfs_dqblk_verify(mp, &dqb[i], id + i, 0);
>  		if (fa)
>  			return fa;
>  	}
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index a2f8cb334bb3..1aac52d7fef4 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -153,6 +153,8 @@ typedef uint16_t	xfs_qwarncnt_t;
>  
>  extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
>  		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type);
> +extern xfs_failaddr_t xfs_dqblk_verify(struct xfs_mount *mp,
> +		struct xfs_dqblk *dqb, xfs_dqid_t id, uint type);
>  extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
>  extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
>  		xfs_dqid_t id, uint type);
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 8d378f485260..d0880c1add41 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -953,6 +953,7 @@ xfs_qm_dqflush(
>  {
>  	struct xfs_mount	*mp = dqp->q_mount;
>  	struct xfs_buf		*bp;
> +	struct xfs_dqblk	*dqb;
>  	struct xfs_disk_dquot	*ddqp;
>  	xfs_failaddr_t		fa;
>  	int			error;
> @@ -996,12 +997,13 @@ xfs_qm_dqflush(
>  	/*
>  	 * Calculate the location of the dquot inside the buffer.
>  	 */
> -	ddqp = bp->b_addr + dqp->q_bufoffset;
> +	dqb = bp->b_addr + dqp->q_bufoffset;
> +	ddqp = &dqb->dd_diskdq;
>  
>  	/*
> -	 * A simple sanity check in case we got a corrupted dquot..
> +	 * A simple sanity check in case we got a corrupted dquot.
>  	 */
> -	fa = xfs_dquot_verify(mp, &dqp->q_core, be32_to_cpu(ddqp->d_id), 0);
> +	fa = xfs_dqblk_verify(mp, dqb, be32_to_cpu(ddqp->d_id), 0);
>  	if (fa) {
>  		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
>  				be32_to_cpu(ddqp->d_id), fa);
> @@ -1032,8 +1034,6 @@ xfs_qm_dqflush(
>  	 * of a dquot without an up-to-date CRC getting to disk.
>  	 */
>  	if (xfs_sb_version_hascrc(&mp->m_sb)) {
> -		struct xfs_dqblk *dqb = (struct xfs_dqblk *)ddqp;
> -
>  		dqb->dd_lsn = cpu_to_be64(dqp->q_logitem.qli_item.li_lsn);
>  		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
>  				 XFS_DQUOT_CRC_OFF);
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 6690b938d450..3138c06b68a6 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -865,7 +865,7 @@ xfs_qm_reset_dqcounts(
>  		 * find uninitialised dquot blks. See comment in
>  		 * xfs_dquot_verify.
>  		 */
> -		fa = xfs_dquot_verify(mp, ddq, id + j, type);
> +		fa = xfs_dqblk_verify(mp, &dqb[j], id + j, type);
>  		if (fa)
>  			xfs_dqblk_repair(mp, &dqb[j], id + j, type);
>  
> -- 
> 2.17.0
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] xfs: add full xfs_dqblk verifier

[Christoph Hellwig]

On Fri, May 04, 2018 at 12:35:05PM -0500, Eric Sandeen wrote:
> Add an xfs_dqblk verifier so that it can check the uuid on V5 filesystems;
> it calls the existing xfs_dquot_verify verifier to validate the
> xfs_disk_dquot_t contained inside it.  This lets us move the uuid
> verification out of the crc verifier, which makes little sense.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_dquot_buf.c  | 27 ++++++++++++++++++++++-----
>  fs/xfs/libxfs/xfs_quota_defs.h |  2 ++
>  fs/xfs/xfs_dquot.c             | 10 +++++-----
>  fs/xfs/xfs_qm.c                |  2 +-
>  4 files changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index 3b92427883fa..d71c0b53536b 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -41,7 +41,12 @@ xfs_calc_dquots_per_chunk(
>  
>  /*
>   * Do some primitive error checking on ondisk dquot data structures.
> + *
> + * The xfs_dqblk structure /contains/ the xfs_disk_dquot structure;
> + * we verify them separately because at some points we have only the
> + * smaller xfs_disk_dquot structure available.
>   */
> +
>  xfs_failaddr_t
>  xfs_dquot_verify(
>  	struct xfs_mount *mp,
> @@ -100,6 +105,20 @@ xfs_dquot_verify(
>  	return NULL;
>  }
>  
> +xfs_failaddr_t
> +xfs_dqblk_verify(
> +	struct xfs_mount	*mp,
> +	struct xfs_dqblk	*dqb,
> +	xfs_dqid_t	 	id,
> +	uint		 	type)	/* used only during quotacheck */
> +{
> +	if (xfs_sb_version_hascrc(&mp->m_sb) &&
> +	    !uuid_equal(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid))
> +			return __this_address;

Double indentation for the return statement.

>  	struct xfs_mount	*mp,
>  	struct xfs_buf		*bp)
>  {
> -	struct xfs_dqblk	*d = (struct xfs_dqblk *)bp->b_addr;
> +	struct xfs_dqblk	*dqb = (struct xfs_dqblk *)bp->b_addr;

No need for the cast, b_addr is a void pointer.

Otherwise looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 4/5 V2] xfs: add full xfs_dqblk verifier

[Eric Sandeen]

Add an xfs_dqblk verifier so that it can check the uuid on V5 filesystems;
it calls the existing xfs_dquot_verify verifier to validate the
xfs_disk_dquot_t contained inside it.  This lets us move the uuid
verification out of the crc verifier, which makes little sense.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---

V2: Fix extra indent and unneeded cast.

 fs/xfs/libxfs/xfs_dquot_buf.c  | 27 ++++++++++++++++++++++-----
 fs/xfs/libxfs/xfs_quota_defs.h |  2 ++
 fs/xfs/xfs_dquot.c             | 10 +++++-----
 fs/xfs/xfs_qm.c                |  2 +-
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 3b92427883fa..d71c0b53536b 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -41,7 +41,12 @@ xfs_calc_dquots_per_chunk(
 
 /*
  * Do some primitive error checking on ondisk dquot data structures.
+ *
+ * The xfs_dqblk structure /contains/ the xfs_disk_dquot structure;
+ * we verify them separately because at some points we have only the
+ * smaller xfs_disk_dquot structure available.
  */
+
 xfs_failaddr_t
 xfs_dquot_verify(
 	struct xfs_mount *mp,
@@ -100,6 +105,20 @@ xfs_dquot_verify(
 	return NULL;
 }
 
+xfs_failaddr_t
+xfs_dqblk_verify(
+	struct xfs_mount	*mp,
+	struct xfs_dqblk	*dqb,
+	xfs_dqid_t	 	id,
+	uint		 	type)	/* used only during quotacheck */
+{
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	    !uuid_equal(&dqb->dd_uuid, &mp->m_sb.sb_meta_uuid))
+		return __this_address;
+
+	return xfs_dquot_verify(mp, &dqb->dd_diskdq, id, type);
+}
+
 /*
  * Do some primitive error checking on ondisk dquot data structures.
  */
@@ -156,8 +175,6 @@ xfs_dquot_buf_verify_crc(
 		if (!xfs_verify_cksum((char *)d, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF))
 			return false;
-		if (!uuid_equal(&d->dd_uuid, &mp->m_sb.sb_meta_uuid))
-			return false;
 	}
 	return true;
 }
@@ -167,7 +184,7 @@ xfs_dquot_buf_verify(
 	struct xfs_mount	*mp,
 	struct xfs_buf		*bp)
 {
-	struct xfs_dqblk	*d = (struct xfs_dqblk *)bp->b_addr;
+	struct xfs_dqblk	*dqb = bp->b_addr;
 	xfs_failaddr_t		fa;
 	xfs_dqid_t		id = 0;
 	int			ndquots;
@@ -193,12 +210,12 @@ xfs_dquot_buf_verify(
 	for (i = 0; i < ndquots; i++) {
 		struct xfs_disk_dquot	*ddq;
 
-		ddq = &d[i].dd_diskdq;
+		ddq = &dqb[i].dd_diskdq;
 
 		if (i == 0)
 			id = be32_to_cpu(ddq->d_id);
 
-		fa = xfs_dquot_verify(mp, ddq, id + i, 0);
+		fa = xfs_dqblk_verify(mp, &dqb[i], id + i, 0);
 		if (fa)
 			return fa;
 	}
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index a2f8cb334bb3..1aac52d7fef4 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -153,6 +153,8 @@ typedef uint16_t	xfs_qwarncnt_t;
 
 extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
 		struct xfs_disk_dquot *ddq, xfs_dqid_t id, uint type);
+extern xfs_failaddr_t xfs_dqblk_verify(struct xfs_mount *mp,
+		struct xfs_dqblk *dqb, xfs_dqid_t id, uint type);
 extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
 extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
 		xfs_dqid_t id, uint type);
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 8d378f485260..d0880c1add41 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -953,6 +953,7 @@ xfs_qm_dqflush(
 {
 	struct xfs_mount	*mp = dqp->q_mount;
 	struct xfs_buf		*bp;
+	struct xfs_dqblk	*dqb;
 	struct xfs_disk_dquot	*ddqp;
 	xfs_failaddr_t		fa;
 	int			error;
@@ -996,12 +997,13 @@ xfs_qm_dqflush(
 	/*
 	 * Calculate the location of the dquot inside the buffer.
 	 */
-	ddqp = bp->b_addr + dqp->q_bufoffset;
+	dqb = bp->b_addr + dqp->q_bufoffset;
+	ddqp = &dqb->dd_diskdq;
 
 	/*
-	 * A simple sanity check in case we got a corrupted dquot..
+	 * A simple sanity check in case we got a corrupted dquot.
 	 */
-	fa = xfs_dquot_verify(mp, &dqp->q_core, be32_to_cpu(ddqp->d_id), 0);
+	fa = xfs_dqblk_verify(mp, dqb, be32_to_cpu(ddqp->d_id), 0);
 	if (fa) {
 		xfs_alert(mp, "corrupt dquot ID 0x%x in memory at %pS",
 				be32_to_cpu(ddqp->d_id), fa);
@@ -1032,8 +1034,6 @@ xfs_qm_dqflush(
 	 * of a dquot without an up-to-date CRC getting to disk.
 	 */
 	if (xfs_sb_version_hascrc(&mp->m_sb)) {
-		struct xfs_dqblk *dqb = (struct xfs_dqblk *)ddqp;
-
 		dqb->dd_lsn = cpu_to_be64(dqp->q_logitem.qli_item.li_lsn);
 		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 6690b938d450..3138c06b68a6 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -865,7 +865,7 @@ xfs_qm_reset_dqcounts(
 		 * find uninitialised dquot blks. See comment in
 		 * xfs_dquot_verify.
 		 */
-		fa = xfs_dquot_verify(mp, ddq, id + j, type);
+		fa = xfs_dqblk_verify(mp, &dqb[j], id + j, type);
 		if (fa)
 			xfs_dqblk_repair(mp, &dqb[j], id + j, type);
 
-- 
2.17.0


--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] xfs: print specific dqblk that failed verifiers

[Eric Sandeen]

Rather than printing the top of the buffer that held a corrupted dqblk,
restructure things to print out the specific one that failed by pushing
the calls to the verifier_error function down into the verifier which
iterates over the buffer and detects the error.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_dquot_buf.c | 41 +++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index d71c0b53536b..fb23d941e799 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -152,7 +152,8 @@ xfs_dqblk_repair(
 STATIC bool
 xfs_dquot_buf_verify_crc(
 	struct xfs_mount	*mp,
-	struct xfs_buf		*bp)
+	struct xfs_buf		*bp,
+	bool			readahead)
 {
 	struct xfs_dqblk	*d = (struct xfs_dqblk *)bp->b_addr;
 	int			ndquots;
@@ -173,8 +174,12 @@ xfs_dquot_buf_verify_crc(
 
 	for (i = 0; i < ndquots; i++, d++) {
 		if (!xfs_verify_cksum((char *)d, sizeof(struct xfs_dqblk),
-				 XFS_DQUOT_CRC_OFF))
+				 XFS_DQUOT_CRC_OFF)) {
+			if (!readahead)
+				xfs_buf_verifier_error(bp, -EFSBADCRC, __func__,
+					d, sizeof(*d), __this_address);
 			return false;
+		}
 	}
 	return true;
 }
@@ -182,7 +187,8 @@ xfs_dquot_buf_verify_crc(
 STATIC xfs_failaddr_t
 xfs_dquot_buf_verify(
 	struct xfs_mount	*mp,
-	struct xfs_buf		*bp)
+	struct xfs_buf		*bp,
+	bool			readahead)
 {
 	struct xfs_dqblk	*dqb = (struct xfs_dqblk *)bp->b_addr;
 	xfs_failaddr_t		fa;
@@ -216,8 +222,13 @@ xfs_dquot_buf_verify(
 			id = be32_to_cpu(ddq->d_id);
 
 		fa = xfs_dqblk_verify(mp, &dqb[i], id + i, 0);
-		if (fa)
+		if (fa) {
+			if (!readahead)
+				xfs_buf_verifier_error(bp, -EFSCORRUPTED,
+					__func__, &dqb[i],
+					sizeof(struct xfs_dqblk), fa);
 			return fa;
+		}
 	}
 
 	return NULL;
@@ -229,7 +240,7 @@ xfs_dquot_buf_verify_struct(
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	return xfs_dquot_buf_verify(mp, bp);
+	return xfs_dquot_buf_verify(mp, bp, false);
 }
 
 static void
@@ -237,15 +248,10 @@ xfs_dquot_buf_read_verify(
 	struct xfs_buf		*bp)
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
-	xfs_failaddr_t		fa;
 
-	if (!xfs_dquot_buf_verify_crc(mp, bp))
-		xfs_verifier_error(bp, -EFSBADCRC, __this_address);
-	else {
-		fa = xfs_dquot_buf_verify(mp, bp);
-		if (fa)
-			xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
-	}
+	if (!xfs_dquot_buf_verify_crc(mp, bp, false))
+		return;
+	xfs_dquot_buf_verify(mp, bp, false);
 }
 
 /*
@@ -260,8 +266,8 @@ xfs_dquot_buf_readahead_verify(
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if (!xfs_dquot_buf_verify_crc(mp, bp) ||
-	    xfs_dquot_buf_verify(mp, bp) != NULL) {
+	if (!xfs_dquot_buf_verify_crc(mp, bp, true) ||
+	    xfs_dquot_buf_verify(mp, bp, true) != NULL) {
 		xfs_buf_ioerror(bp, -EIO);
 		bp->b_flags &= ~XBF_DONE;
 	}
@@ -277,11 +283,8 @@ xfs_dquot_buf_write_verify(
 	struct xfs_buf		*bp)
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
-	xfs_failaddr_t		fa;
 
-	fa = xfs_dquot_buf_verify(mp, bp);
-	if (fa)
-		xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
+	xfs_dquot_buf_verify(mp, bp, false);
 }
 
 const struct xfs_buf_ops xfs_dquot_buf_ops = {
-- 
2.17.0

[PATCH 6/5] xfs: make several functions void

[Eric Sandeen]

Several functions can return only zero, and so we can change
their type to void and remove error handling.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Adding this as 6/5 just because it hits my other quota changes,
I can rebase as needed.

 libxfs/xfs_attr_leaf.c  |   14 +++++++-------
 libxfs/xfs_dir2.c       |    4 ++--
 libxfs/xfs_dir2_priv.h  |    2 +-
 libxfs/xfs_dir2_sf.c    |    3 +--
 libxfs/xfs_dquot_buf.c  |    4 +---
 libxfs/xfs_quota_defs.h |    2 +-
 libxfs/xfs_sb.c         |   10 +++++-----
 libxfs/xfs_sb.h         |    2 +-
 xfs_dir2_readdir.c      |   15 +++++++--------
 xfs_fsops.c             |    3 +--
 xfs_fsops.h             |    2 +-
 xfs_inode.c             |   32 +++++++++-----------------------
 xfs_ioctl.c             |   15 +++------------
 xfs_ioctl32.c           |    6 ++----
 xfs_log.c               |   26 ++++++++------------------
 xfs_log_recover.c       |   41 ++++++++++++++++-------------------------
 xfs_super.c             |    7 +++----
 17 files changed, 69 insertions(+), 119 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 2135b8e67dcc..73f2c1ae2dcf 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -59,7 +59,7 @@
  */
 STATIC int xfs_attr3_leaf_create(struct xfs_da_args *args,
 				 xfs_dablk_t which_block, struct xfs_buf **bpp);
-STATIC int xfs_attr3_leaf_add_work(struct xfs_buf *leaf_buffer,
+STATIC void xfs_attr3_leaf_add_work(struct xfs_buf *leaf_buffer,
 				   struct xfs_attr3_icleaf_hdr *ichdr,
 				   struct xfs_da_args *args, int freemap_index);
 STATIC void xfs_attr3_leaf_compact(struct xfs_da_args *args,
@@ -1247,6 +1247,7 @@ xfs_attr3_leaf_add(
 	int			entsize;
 	int			sum;
 	int			tmp;
+	int			error = 0;
 	int			i;
 
 	trace_xfs_attr_leaf_add(args);
@@ -1273,7 +1274,7 @@ xfs_attr3_leaf_add(
 		if (ichdr.freemap[i].base < ichdr.firstused)
 			tmp += sizeof(xfs_attr_leaf_entry_t);
 		if (ichdr.freemap[i].size >= tmp) {
-			tmp = xfs_attr3_leaf_add_work(bp, &ichdr, args, i);
+			xfs_attr3_leaf_add_work(bp, &ichdr, args, i);
 			goto out_log_hdr;
 		}
 		sum += ichdr.freemap[i].size;
@@ -1298,24 +1299,24 @@ xfs_attr3_leaf_add(
 	 * free region, in freemap[0].  If it is not big enough, give up.
 	 */
 	if (ichdr.freemap[0].size < (entsize + sizeof(xfs_attr_leaf_entry_t))) {
-		tmp = -ENOSPC;
+		error = -ENOSPC;
 		goto out_log_hdr;
 	}
 
-	tmp = xfs_attr3_leaf_add_work(bp, &ichdr, args, 0);
+	xfs_attr3_leaf_add_work(bp, &ichdr, args, 0);
 
 out_log_hdr:
 	xfs_attr3_leaf_hdr_to_disk(args->geo, leaf, &ichdr);
 	xfs_trans_log_buf(args->trans, bp,
 		XFS_DA_LOGRANGE(leaf, &leaf->hdr,
 				xfs_attr3_leaf_hdr_size(leaf)));
-	return tmp;
+	return error;
 }
 
 /*
  * Add a name to a leaf attribute list structure.
  */
-STATIC int
+STATIC void
 xfs_attr3_leaf_add_work(
 	struct xfs_buf		*bp,
 	struct xfs_attr3_icleaf_hdr *ichdr,
@@ -1429,7 +1430,6 @@ xfs_attr3_leaf_add_work(
 		}
 	}
 	ichdr->usedbytes += xfs_attr_leaf_entsize(leaf, args->index);
-	return 0;
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 92f94e190f04..bf1044aa236f 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -240,9 +240,9 @@ xfs_dir_init(
 	args->geo = dp->i_mount->m_dir_geo;
 	args->dp = dp;
 	args->trans = tp;
-	error = xfs_dir2_sf_create(args, pdp->i_ino);
+	xfs_dir2_sf_create(args, pdp->i_ino);
 	kmem_free(args);
-	return error;
+	return 0;
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
index 753aeeeffc18..f51a3d9924e7 100644
--- a/fs/xfs/libxfs/xfs_dir2_priv.h
+++ b/fs/xfs/libxfs/xfs_dir2_priv.h
@@ -125,7 +125,7 @@ extern int xfs_dir2_block_sfsize(struct xfs_inode *dp,
 extern int xfs_dir2_block_to_sf(struct xfs_da_args *args, struct xfs_buf *bp,
 		int size, xfs_dir2_sf_hdr_t *sfhp);
 extern int xfs_dir2_sf_addname(struct xfs_da_args *args);
-extern int xfs_dir2_sf_create(struct xfs_da_args *args, xfs_ino_t pino);
+extern void xfs_dir2_sf_create(struct xfs_da_args *args, xfs_ino_t pino);
 extern int xfs_dir2_sf_lookup(struct xfs_da_args *args);
 extern int xfs_dir2_sf_removename(struct xfs_da_args *args);
 extern int xfs_dir2_sf_replace(struct xfs_da_args *args);
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index 0c75a7f00883..eb34948b6aef 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -736,7 +736,7 @@ xfs_dir2_sf_verify(
 /*
  * Create a new (shortform) directory.
  */
-int					/* error, always 0 */
+void					/* always succeeds */
 xfs_dir2_sf_create(
 	xfs_da_args_t	*args,		/* operation arguments */
 	xfs_ino_t	pino)		/* parent inode number */
@@ -783,7 +783,6 @@ xfs_dir2_sf_create(
 	dp->i_d.di_size = size;
 	xfs_dir2_sf_check(args);
 	xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE | XFS_ILOG_DDATA);
-	return 0;
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index fb23d941e799..f3a7a2d5690e 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -122,7 +122,7 @@ xfs_dqblk_verify(
 /*
  * Do some primitive error checking on ondisk dquot data structures.
  */
-int
+void
 xfs_dqblk_repair(
 	struct xfs_mount	*mp,
 	struct xfs_dqblk	*dqb,
@@ -145,8 +145,6 @@ xfs_dqblk_repair(
 		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
 	}
-
-	return 0;
 }
 
 STATIC bool
diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index 1aac52d7fef4..7ae95e7643e6 100644
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -156,7 +156,7 @@ extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
 extern xfs_failaddr_t xfs_dqblk_verify(struct xfs_mount *mp,
 		struct xfs_dqblk *dqb, xfs_dqid_t id, uint type);
 extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
-extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
+extern void xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
 		xfs_dqid_t id, uint type);
 
 #endif	/* __XFS_QUOTA_H__ */
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index d9b94bd5f689..7c578740e57d 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -888,7 +888,7 @@ xfs_sync_sb(
 	return xfs_trans_commit(tp);
 }
 
-int
+void
 xfs_fs_geometry(
 	struct xfs_sb		*sbp,
 	struct xfs_fsop_geom	*geo,
@@ -912,13 +912,13 @@ xfs_fs_geometry(
 	memcpy(geo->uuid, &sbp->sb_uuid, sizeof(sbp->sb_uuid));
 
 	if (struct_version < 2)
-		return 0;
+		return;
 
 	geo->sunit = sbp->sb_unit;
 	geo->swidth = sbp->sb_width;
 
 	if (struct_version < 3)
-		return 0;
+		return;
 
 	geo->version = XFS_FSOP_GEOM_VERSION;
 	geo->flags = XFS_FSOP_GEOM_FLAGS_NLINK |
@@ -963,12 +963,12 @@ xfs_fs_geometry(
 	geo->dirblocksize = xfs_dir2_dirblock_bytes(sbp);
 
 	if (struct_version < 4)
-		return 0;
+		return;
 
 	if (xfs_sb_version_haslogv2(sbp))
 		geo->flags |= XFS_FSOP_GEOM_FLAGS_LOGV2;
 
 	geo->logsunit = sbp->sb_logsunit;
 
-	return 0;
+	return;
 }
diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
index 63dcd2a1a657..cf384d584170 100644
--- a/fs/xfs/libxfs/xfs_sb.h
+++ b/fs/xfs/libxfs/xfs_sb.h
@@ -35,7 +35,7 @@ extern void	xfs_sb_to_disk(struct xfs_dsb *to, struct xfs_sb *from);
 extern void	xfs_sb_quota_from_disk(struct xfs_sb *sbp);
 
 #define XFS_FS_GEOM_MAX_STRUCT_VER	(4)
-extern int	xfs_fs_geometry(struct xfs_sb *sbp, struct xfs_fsop_geom *geo,
+extern void	xfs_fs_geometry(struct xfs_sb *sbp, struct xfs_fsop_geom *geo,
 				int struct_version);
 
 #endif	/* __XFS_SB_H__ */
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index b6ae3597bfb0..c9ed5b6cb391 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -55,7 +55,7 @@ xfs_dir3_get_dtype(
 	return xfs_dir3_filetype_table[filetype];
 }
 
-STATIC int
+STATIC void
 xfs_dir2_sf_getdents(
 	struct xfs_da_args	*args,
 	struct dir_context	*ctx)
@@ -80,7 +80,7 @@ xfs_dir2_sf_getdents(
 	 * If the block number in the offset is out of range, we're done.
 	 */
 	if (xfs_dir2_dataptr_to_db(geo, ctx->pos) > geo->datablk)
-		return 0;
+		return;
 
 	/*
 	 * Precalculate offsets for . and .. as we will always need them.
@@ -99,7 +99,7 @@ xfs_dir2_sf_getdents(
 	if (ctx->pos <= dot_offset) {
 		ctx->pos = dot_offset & 0x7fffffff;
 		if (!dir_emit(ctx, ".", 1, dp->i_ino, DT_DIR))
-			return 0;
+			return;
 	}
 
 	/*
@@ -109,7 +109,7 @@ xfs_dir2_sf_getdents(
 		ino = dp->d_ops->sf_get_parent_ino(sfp);
 		ctx->pos = dotdot_offset & 0x7fffffff;
 		if (!dir_emit(ctx, "..", 2, ino, DT_DIR))
-			return 0;
+			return;
 	}
 
 	/*
@@ -132,13 +132,12 @@ xfs_dir2_sf_getdents(
 		ctx->pos = off & 0x7fffffff;
 		if (!dir_emit(ctx, (char *)sfep->name, sfep->namelen, ino,
 			    xfs_dir3_get_dtype(dp->i_mount, filetype)))
-			return 0;
+			return;
 		sfep = dp->d_ops->sf_nextentry(sfp, sfep);
 	}
 
 	ctx->pos = xfs_dir2_db_off_to_dataptr(geo, geo->datablk + 1, 0) &
 								0x7fffffff;
-	return 0;
 }
 
 /*
@@ -513,7 +512,7 @@ xfs_readdir(
 	size_t			bufsize)
 {
 	struct xfs_da_args	args = { NULL };
-	int			rval;
+	int			rval = 0;
 	int			v;
 
 	trace_xfs_readdir(dp);
@@ -529,7 +528,7 @@ xfs_readdir(
 	args.trans = tp;
 
 	if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
-		rval = xfs_dir2_sf_getdents(&args, ctx);
+		xfs_dir2_sf_getdents(&args, ctx);
 	else if ((rval = xfs_dir2_isblock(&args, &v)))
 		;
 	else if (v)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 523792768080..2647a6ee7d79 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -687,7 +687,7 @@ xfs_growfs_log(
  * exported through ioctl XFS_IOC_FSCOUNTS
  */
 
-int
+void
 xfs_fs_counts(
 	xfs_mount_t		*mp,
 	xfs_fsop_counts_t	*cnt)
@@ -700,7 +700,6 @@ xfs_fs_counts(
 	spin_lock(&mp->m_sb_lock);
 	cnt->freertx = mp->m_sb.sb_frextents;
 	spin_unlock(&mp->m_sb_lock);
-	return 0;
 }
 
 /*
diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
index 20484ed5e919..2108f2191daf 100644
--- a/fs/xfs/xfs_fsops.h
+++ b/fs/xfs/xfs_fsops.h
@@ -20,7 +20,7 @@
 
 extern int xfs_growfs_data(xfs_mount_t *mp, xfs_growfs_data_t *in);
 extern int xfs_growfs_log(xfs_mount_t *mp, xfs_growfs_log_t *in);
-extern int xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
+extern void xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
 extern int xfs_reserve_blocks(xfs_mount_t *mp, uint64_t *inval,
 				xfs_fsop_resblks_t *outval);
 extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 2b70c8b4cee2..3a9b193a7e8d 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1116,7 +1116,7 @@ xfs_droplink(
 /*
  * Increment the link count on an inode & log the change.
  */
-static int
+static void 
 xfs_bumplink(
 	xfs_trans_t *tp,
 	xfs_inode_t *ip)
@@ -1126,7 +1126,6 @@ xfs_bumplink(
 	ASSERT(ip->i_d.di_version > 1);
 	inc_nlink(VFS_I(ip));
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-	return 0;
 }
 
 int
@@ -1238,10 +1237,7 @@ xfs_create(
 		error = xfs_dir_init(tp, ip, dp);
 		if (error)
 			goto out_bmap_cancel;
-
-		error = xfs_bumplink(tp, dp);
-		if (error)
-			goto out_bmap_cancel;
+		xfs_bumplink(tp, dp);
 	}
 
 	/*
@@ -1468,9 +1464,7 @@ xfs_link(
 	xfs_trans_ichgtime(tp, tdp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
 	xfs_trans_log_inode(tp, tdp, XFS_ILOG_CORE);
 
-	error = xfs_bumplink(tp, sip);
-	if (error)
-		goto error_return;
+	xfs_bumplink(tp, sip);
 
 	/*
 	 * If this is a synchronous mount, make sure that the
@@ -2815,9 +2809,7 @@ xfs_cross_rename(
 				error = xfs_droplink(tp, dp2);
 				if (error)
 					goto out_trans_abort;
-				error = xfs_bumplink(tp, dp1);
-				if (error)
-					goto out_trans_abort;
+				xfs_bumplink(tp, dp1);
 			}
 
 			/*
@@ -2842,9 +2834,7 @@ xfs_cross_rename(
 				error = xfs_droplink(tp, dp1);
 				if (error)
 					goto out_trans_abort;
-				error = xfs_bumplink(tp, dp2);
-				if (error)
-					goto out_trans_abort;
+				xfs_bumplink(tp, dp2);
 			}
 
 			/*
@@ -3048,11 +3038,9 @@ xfs_rename(
 		xfs_trans_ichgtime(tp, target_dp,
 					XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
 
-		if (new_parent && src_is_directory) {
-			error = xfs_bumplink(tp, target_dp);
-			if (error)
-				goto out_bmap_cancel;
-		}
+		if (new_parent && src_is_directory)
+			xfs_bumplink(tp, target_dp);
+
 	} else { /* target_ip != NULL */
 		/*
 		 * If target exists and it's a directory, check that both
@@ -3172,9 +3160,7 @@ xfs_rename(
 	 */
 	if (wip) {
 		ASSERT(VFS_I(wip)->i_nlink == 0);
-		error = xfs_bumplink(tp, wip);
-		if (error)
-			goto out_bmap_cancel;
+		xfs_bumplink(tp, wip);
 		error = xfs_iunlink_remove(tp, wip);
 		if (error)
 			goto out_bmap_cancel;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 89fb1eb80aae..4ba8939145b8 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -808,11 +808,8 @@ xfs_ioc_fsgeometry_v1(
 	void			__user *arg)
 {
 	xfs_fsop_geom_t         fsgeo;
-	int			error;
 
-	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
-	if (error)
-		return error;
+	xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
 
 	/*
 	 * Caller should have passed an argument of type
@@ -830,11 +827,8 @@ xfs_ioc_fsgeometry(
 	void			__user *arg)
 {
 	xfs_fsop_geom_t		fsgeo;
-	int			error;
 
-	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 4);
-	if (error)
-		return error;
+	xfs_fs_geometry(&mp->m_sb, &fsgeo, 4);
 
 	if (copy_to_user(arg, &fsgeo, sizeof(fsgeo)))
 		return -EFAULT;
@@ -1963,10 +1957,7 @@ xfs_file_ioctl(
 	case XFS_IOC_FSCOUNTS: {
 		xfs_fsop_counts_t out;
 
-		error = xfs_fs_counts(mp, &out);
-		if (error)
-			return error;
-
+		xfs_fs_counts(mp, &out);
 		if (copy_to_user(arg, &out, sizeof(out)))
 			return -EFAULT;
 		return 0;
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 10fbde359649..bb868ca31319 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -65,11 +65,9 @@ xfs_compat_ioc_fsgeometry_v1(
 	compat_xfs_fsop_geom_v1_t __user *arg32)
 {
 	xfs_fsop_geom_t		  fsgeo;
-	int			  error;
 
-	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
-	if (error)
-		return error;
+	xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
+
 	/* The 32-bit variant simply has some padding at the end */
 	if (copy_to_user(arg32, &fsgeo, sizeof(struct compat_xfs_fsop_geom_v1)))
 		return -EFAULT;
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 2fcd9ed5d075..65121e587e95 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -56,7 +56,7 @@ STATIC int
 xlog_space_left(
 	struct xlog		*log,
 	atomic64_t		*head);
-STATIC int
+STATIC void
 xlog_sync(
 	struct xlog		*log,
 	struct xlog_in_core	*iclog);
@@ -1769,7 +1769,7 @@ xlog_cksum(
  * ensure that completes before tearing down the iclogbufs. Hence we need to
  * hold the buffer lock across the log IO to acheive that.
  */
-STATIC int
+STATIC void
 xlog_bdstrat(
 	struct xfs_buf		*bp)
 {
@@ -1786,11 +1786,10 @@ xlog_bdstrat(
 		 * doing it here. Similarly, IO completion will unlock the
 		 * buffer, so we don't do it here.
 		 */
-		return 0;
+		return;
 	}
 
 	xfs_buf_submit(bp);
-	return 0;
 }
 
 /*
@@ -1818,7 +1817,7 @@ xlog_bdstrat(
  * is added immediately before calling bwrite().
  */
 
-STATIC int
+STATIC void
 xlog_sync(
 	struct xlog		*log,
 	struct xlog_in_core	*iclog)
@@ -1829,7 +1828,6 @@ xlog_sync(
 	uint		count_init;	/* initial count before roundup */
 	int		roundoff;       /* roundoff to BB or stripe */
 	int		split = 0;	/* split write into two regions */
-	int		error;
 	int		v2 = xfs_sb_version_haslogv2(&log->l_mp->m_sb);
 	int		size;
 
@@ -1948,11 +1946,8 @@ xlog_sync(
 	 * Don't call xfs_bwrite here. We do log-syncs even when the filesystem
 	 * is shutting down.
 	 */
-	error = xlog_bdstrat(bp);
-	if (error) {
-		xfs_buf_ioerror_alert(bp, "xlog_sync");
-		return error;
-	}
+	xlog_bdstrat(bp);
+
 	if (split) {
 		bp = iclog->ic_log->l_xbuf;
 		XFS_BUF_SET_ADDR(bp, 0);	     /* logical 0 */
@@ -1967,13 +1962,8 @@ xlog_sync(
 
 		/* account for internal log which doesn't start at block #0 */
 		XFS_BUF_SET_ADDR(bp, XFS_BUF_ADDR(bp) + log->l_logBBstart);
-		error = xlog_bdstrat(bp);
-		if (error) {
-			xfs_buf_ioerror_alert(bp, "xlog_sync (split)");
-			return error;
-		}
+		xlog_bdstrat(bp);
 	}
-	return 0;
 }	/* xlog_sync */
 
 /*
@@ -3220,7 +3210,7 @@ xlog_state_release_iclog(
 	 * flags after this point.
 	 */
 	if (sync)
-		return xlog_sync(log, iclog);
+		xlog_sync(log, iclog);
 	return 0;
 }	/* xlog_state_release_iclog */
 
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 06a09cb948b5..a0f11bab3add 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2011,7 +2011,7 @@ xlog_recover_reorder_trans(
  * record in the table to tell us how many times we expect to see this
  * record during the second pass.
  */
-STATIC int
+STATIC void
 xlog_recover_buffer_pass1(
 	struct xlog			*log,
 	struct xlog_recover_item	*item)
@@ -2025,7 +2025,7 @@ xlog_recover_buffer_pass1(
 	 */
 	if (!(buf_f->blf_flags & XFS_BLF_CANCEL)) {
 		trace_xfs_log_recover_buf_not_cancel(log, buf_f);
-		return 0;
+		return;
 	}
 
 	/*
@@ -2038,7 +2038,7 @@ xlog_recover_buffer_pass1(
 		    bcp->bc_len == buf_f->blf_len) {
 			bcp->bc_refcount++;
 			trace_xfs_log_recover_buf_cancel_ref_inc(log, buf_f);
-			return 0;
+			return;
 		}
 	}
 
@@ -2049,7 +2049,6 @@ xlog_recover_buffer_pass1(
 	list_add_tail(&bcp->bc_list, bucket);
 
 	trace_xfs_log_recover_buf_cancel_add(log, buf_f);
-	return 0;
 }
 
 /*
@@ -3270,7 +3269,7 @@ xlog_recover_inode_pass2(
  * structure, so that we know not to do any dquot item or dquot buffer recovery,
  * of that type.
  */
-STATIC int
+STATIC void
 xlog_recover_quotaoff_pass1(
 	struct xlog			*log,
 	struct xlog_recover_item	*item)
@@ -3288,8 +3287,6 @@ xlog_recover_quotaoff_pass1(
 		log->l_quotaoffs_flag |= XFS_DQ_PROJ;
 	if (qoff_f->qf_flags & XFS_GQUOTA_ACCT)
 		log->l_quotaoffs_flag |= XFS_DQ_GROUP;
-
-	return 0;
 }
 
 /*
@@ -3449,7 +3446,7 @@ xlog_recover_efi_pass2(
  * equal to that in the EFD format structure. If we find it we drop the EFD
  * reference, which removes the EFI from the AIL and frees it.
  */
-STATIC int
+STATIC void
 xlog_recover_efd_pass2(
 	struct xlog			*log,
 	struct xlog_recover_item	*item)
@@ -3493,8 +3490,6 @@ xlog_recover_efd_pass2(
 
 	xfs_trans_ail_cursor_done(&cur);
 	spin_unlock(&ailp->ail_lock);
-
-	return 0;
 }
 
 /*
@@ -3545,7 +3540,7 @@ xlog_recover_rui_pass2(
  * equal to that in the RUD format structure. If we find it we drop the RUD
  * reference, which removes the RUI from the AIL and frees it.
  */
-STATIC int
+STATIC void
 xlog_recover_rud_pass2(
 	struct xlog			*log,
 	struct xlog_recover_item	*item)
@@ -3586,8 +3581,6 @@ xlog_recover_rud_pass2(
 
 	xfs_trans_ail_cursor_done(&cur);
 	spin_unlock(&ailp->ail_lock);
-
-	return 0;
 }
 
 /*
@@ -4072,9 +4065,11 @@ xlog_recover_commit_pass1(
 
 	switch (ITEM_TYPE(item)) {
 	case XFS_LI_BUF:
-		return xlog_recover_buffer_pass1(log, item);
+		xlog_recover_buffer_pass1(log, item);
+		return 0;
 	case XFS_LI_QUOTAOFF:
-		return xlog_recover_quotaoff_pass1(log, item);
+		xlog_recover_quotaoff_pass1(log, item);
+		return 0;
 	case XFS_LI_INODE:
 	case XFS_LI_EFI:
 	case XFS_LI_EFD:
@@ -4115,11 +4110,13 @@ xlog_recover_commit_pass2(
 	case XFS_LI_EFI:
 		return xlog_recover_efi_pass2(log, item, trans->r_lsn);
 	case XFS_LI_EFD:
-		return xlog_recover_efd_pass2(log, item);
+		xlog_recover_efd_pass2(log, item);
+		return 0;
 	case XFS_LI_RUI:
 		return xlog_recover_rui_pass2(log, item, trans->r_lsn);
 	case XFS_LI_RUD:
-		return xlog_recover_rud_pass2(log, item);
+		xlog_recover_rud_pass2(log, item);
+		return 0;
 	case XFS_LI_CUI:
 		return xlog_recover_cui_pass2(log, item, trans->r_lsn);
 	case XFS_LI_CUD:
@@ -5162,7 +5159,7 @@ xlog_recover_process_iunlinks(
 	}
 }
 
-STATIC int
+STATIC void
 xlog_unpack_data(
 	struct xlog_rec_header	*rhead,
 	char			*dp,
@@ -5185,8 +5182,6 @@ xlog_unpack_data(
 			dp += BBSIZE;
 		}
 	}
-
-	return 0;
 }
 
 /*
@@ -5201,11 +5196,9 @@ xlog_recover_process(
 	int			pass,
 	struct list_head	*buffer_list)
 {
-	int			error;
 	__le32			old_crc = rhead->h_crc;
 	__le32			crc;
 
-
 	crc = xlog_cksum(log, rhead, dp, be32_to_cpu(rhead->h_len));
 
 	/*
@@ -5244,9 +5237,7 @@ xlog_recover_process(
 			return -EFSCORRUPTED;
 	}
 
-	error = xlog_unpack_data(rhead, dp, log);
-	if (error)
-		return error;
+	xlog_unpack_data(rhead, dp, log);
 
 	return xlog_recover_process_data(log, rhash, rhead, dp, pass,
 					 buffer_list);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index d71424052917..efb6c1d5f0bc 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -475,7 +475,7 @@ struct proc_xfs_info {
 	char		*str;
 };
 
-STATIC int
+STATIC void
 xfs_showargs(
 	struct xfs_mount	*mp,
 	struct seq_file		*m)
@@ -555,8 +555,6 @@ xfs_showargs(
 
 	if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
 		seq_puts(m, ",noquota");
-
-	return 0;
 }
 static uint64_t
 xfs_max_file_offset(
@@ -1462,7 +1460,8 @@ xfs_fs_show_options(
 	struct seq_file		*m,
 	struct dentry		*root)
 {
-	return xfs_showargs(XFS_M(root->d_sb), m);
+	xfs_showargs(XFS_M(root->d_sb), m);
+	return 0;
 }
 
 /*

Re: [PATCH 6/5] xfs: make several functions void

[Darrick J. Wong]

> Subject: [PATCH 6/5] xfs: make several functions void

Usual patch cramming practice is to make this a reply to the cover
letter, not 5/5.

Also, "make several functions return void"?

On Fri, May 04, 2018 at 04:46:33PM -0500, Eric Sandeen wrote:
> Several functions can return only zero, and so we can change
> their type to void and remove error handling.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> Adding this as 6/5 just because it hits my other quota changes,
> I can rebase as needed.
> 
>  libxfs/xfs_attr_leaf.c  |   14 +++++++-------
>  libxfs/xfs_dir2.c       |    4 ++--
>  libxfs/xfs_dir2_priv.h  |    2 +-
>  libxfs/xfs_dir2_sf.c    |    3 +--
>  libxfs/xfs_dquot_buf.c  |    4 +---
>  libxfs/xfs_quota_defs.h |    2 +-
>  libxfs/xfs_sb.c         |   10 +++++-----
>  libxfs/xfs_sb.h         |    2 +-
>  xfs_dir2_readdir.c      |   15 +++++++--------
>  xfs_fsops.c             |    3 +--
>  xfs_fsops.h             |    2 +-
>  xfs_inode.c             |   32 +++++++++-----------------------
>  xfs_ioctl.c             |   15 +++------------
>  xfs_ioctl32.c           |    6 ++----
>  xfs_log.c               |   26 ++++++++------------------
>  xfs_log_recover.c       |   41 ++++++++++++++++-------------------------
>  xfs_super.c             |    7 +++----
>  17 files changed, 69 insertions(+), 119 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 2135b8e67dcc..73f2c1ae2dcf 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -59,7 +59,7 @@
>   */
>  STATIC int xfs_attr3_leaf_create(struct xfs_da_args *args,
>  				 xfs_dablk_t which_block, struct xfs_buf **bpp);
> -STATIC int xfs_attr3_leaf_add_work(struct xfs_buf *leaf_buffer,
> +STATIC void xfs_attr3_leaf_add_work(struct xfs_buf *leaf_buffer,

/me hates to be a PITA, but some of these functions ASSERT on on-disk
metadata checks that fail.  Sometimes the checks are for things that are
already caught by the verifiers & so therefore ensure we haven't
corrupted in-core state, but others could very well be things that
should be turned into -EFSCORRUPTED return.

How about splitting these up and only do a few functions (or a single
file) at a time?  (And ideally after I land the series that starts the
conversion of ASSERT-on-disk-corruption to EFSCORRUPTED-return that's
also out for review...)

--D

>  				   struct xfs_attr3_icleaf_hdr *ichdr,
>  				   struct xfs_da_args *args, int freemap_index);
>  STATIC void xfs_attr3_leaf_compact(struct xfs_da_args *args,
> @@ -1247,6 +1247,7 @@ xfs_attr3_leaf_add(
>  	int			entsize;
>  	int			sum;
>  	int			tmp;
> +	int			error = 0;
>  	int			i;
>  
>  	trace_xfs_attr_leaf_add(args);
> @@ -1273,7 +1274,7 @@ xfs_attr3_leaf_add(
>  		if (ichdr.freemap[i].base < ichdr.firstused)
>  			tmp += sizeof(xfs_attr_leaf_entry_t);
>  		if (ichdr.freemap[i].size >= tmp) {
> -			tmp = xfs_attr3_leaf_add_work(bp, &ichdr, args, i);
> +			xfs_attr3_leaf_add_work(bp, &ichdr, args, i);
>  			goto out_log_hdr;
>  		}
>  		sum += ichdr.freemap[i].size;
> @@ -1298,24 +1299,24 @@ xfs_attr3_leaf_add(
>  	 * free region, in freemap[0].  If it is not big enough, give up.
>  	 */
>  	if (ichdr.freemap[0].size < (entsize + sizeof(xfs_attr_leaf_entry_t))) {
> -		tmp = -ENOSPC;
> +		error = -ENOSPC;
>  		goto out_log_hdr;
>  	}
>  
> -	tmp = xfs_attr3_leaf_add_work(bp, &ichdr, args, 0);
> +	xfs_attr3_leaf_add_work(bp, &ichdr, args, 0);
>  
>  out_log_hdr:
>  	xfs_attr3_leaf_hdr_to_disk(args->geo, leaf, &ichdr);
>  	xfs_trans_log_buf(args->trans, bp,
>  		XFS_DA_LOGRANGE(leaf, &leaf->hdr,
>  				xfs_attr3_leaf_hdr_size(leaf)));
> -	return tmp;
> +	return error;
>  }
>  
>  /*
>   * Add a name to a leaf attribute list structure.
>   */
> -STATIC int
> +STATIC void
>  xfs_attr3_leaf_add_work(
>  	struct xfs_buf		*bp,
>  	struct xfs_attr3_icleaf_hdr *ichdr,
> @@ -1429,7 +1430,6 @@ xfs_attr3_leaf_add_work(
>  		}
>  	}
>  	ichdr->usedbytes += xfs_attr_leaf_entsize(leaf, args->index);
> -	return 0;
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index 92f94e190f04..bf1044aa236f 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -240,9 +240,9 @@ xfs_dir_init(
>  	args->geo = dp->i_mount->m_dir_geo;
>  	args->dp = dp;
>  	args->trans = tp;
> -	error = xfs_dir2_sf_create(args, pdp->i_ino);
> +	xfs_dir2_sf_create(args, pdp->i_ino);
>  	kmem_free(args);
> -	return error;
> +	return 0;
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index 753aeeeffc18..f51a3d9924e7 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -125,7 +125,7 @@ extern int xfs_dir2_block_sfsize(struct xfs_inode *dp,
>  extern int xfs_dir2_block_to_sf(struct xfs_da_args *args, struct xfs_buf *bp,
>  		int size, xfs_dir2_sf_hdr_t *sfhp);
>  extern int xfs_dir2_sf_addname(struct xfs_da_args *args);
> -extern int xfs_dir2_sf_create(struct xfs_da_args *args, xfs_ino_t pino);
> +extern void xfs_dir2_sf_create(struct xfs_da_args *args, xfs_ino_t pino);
>  extern int xfs_dir2_sf_lookup(struct xfs_da_args *args);
>  extern int xfs_dir2_sf_removename(struct xfs_da_args *args);
>  extern int xfs_dir2_sf_replace(struct xfs_da_args *args);
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index 0c75a7f00883..eb34948b6aef 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -736,7 +736,7 @@ xfs_dir2_sf_verify(
>  /*
>   * Create a new (shortform) directory.
>   */
> -int					/* error, always 0 */
> +void					/* always succeeds */
>  xfs_dir2_sf_create(
>  	xfs_da_args_t	*args,		/* operation arguments */
>  	xfs_ino_t	pino)		/* parent inode number */
> @@ -783,7 +783,6 @@ xfs_dir2_sf_create(
>  	dp->i_d.di_size = size;
>  	xfs_dir2_sf_check(args);
>  	xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE | XFS_ILOG_DDATA);
> -	return 0;
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index fb23d941e799..f3a7a2d5690e 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -122,7 +122,7 @@ xfs_dqblk_verify(
>  /*
>   * Do some primitive error checking on ondisk dquot data structures.
>   */
> -int
> +void
>  xfs_dqblk_repair(
>  	struct xfs_mount	*mp,
>  	struct xfs_dqblk	*dqb,
> @@ -145,8 +145,6 @@ xfs_dqblk_repair(
>  		xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
>  				 XFS_DQUOT_CRC_OFF);
>  	}
> -
> -	return 0;
>  }
>  
>  STATIC bool
> diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
> index 1aac52d7fef4..7ae95e7643e6 100644
> --- a/fs/xfs/libxfs/xfs_quota_defs.h
> +++ b/fs/xfs/libxfs/xfs_quota_defs.h
> @@ -156,7 +156,7 @@ extern xfs_failaddr_t xfs_dquot_verify(struct xfs_mount *mp,
>  extern xfs_failaddr_t xfs_dqblk_verify(struct xfs_mount *mp,
>  		struct xfs_dqblk *dqb, xfs_dqid_t id, uint type);
>  extern int xfs_calc_dquots_per_chunk(unsigned int nbblks);
> -extern int xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
> +extern void xfs_dqblk_repair(struct xfs_mount *mp, struct xfs_dqblk *dqb,
>  		xfs_dqid_t id, uint type);
>  
>  #endif	/* __XFS_QUOTA_H__ */
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index d9b94bd5f689..7c578740e57d 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -888,7 +888,7 @@ xfs_sync_sb(
>  	return xfs_trans_commit(tp);
>  }
>  
> -int
> +void
>  xfs_fs_geometry(
>  	struct xfs_sb		*sbp,
>  	struct xfs_fsop_geom	*geo,
> @@ -912,13 +912,13 @@ xfs_fs_geometry(
>  	memcpy(geo->uuid, &sbp->sb_uuid, sizeof(sbp->sb_uuid));
>  
>  	if (struct_version < 2)
> -		return 0;
> +		return;
>  
>  	geo->sunit = sbp->sb_unit;
>  	geo->swidth = sbp->sb_width;
>  
>  	if (struct_version < 3)
> -		return 0;
> +		return;
>  
>  	geo->version = XFS_FSOP_GEOM_VERSION;
>  	geo->flags = XFS_FSOP_GEOM_FLAGS_NLINK |
> @@ -963,12 +963,12 @@ xfs_fs_geometry(
>  	geo->dirblocksize = xfs_dir2_dirblock_bytes(sbp);
>  
>  	if (struct_version < 4)
> -		return 0;
> +		return;
>  
>  	if (xfs_sb_version_haslogv2(sbp))
>  		geo->flags |= XFS_FSOP_GEOM_FLAGS_LOGV2;
>  
>  	geo->logsunit = sbp->sb_logsunit;
>  
> -	return 0;
> +	return;
>  }
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index 63dcd2a1a657..cf384d584170 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -35,7 +35,7 @@ extern void	xfs_sb_to_disk(struct xfs_dsb *to, struct xfs_sb *from);
>  extern void	xfs_sb_quota_from_disk(struct xfs_sb *sbp);
>  
>  #define XFS_FS_GEOM_MAX_STRUCT_VER	(4)
> -extern int	xfs_fs_geometry(struct xfs_sb *sbp, struct xfs_fsop_geom *geo,
> +extern void	xfs_fs_geometry(struct xfs_sb *sbp, struct xfs_fsop_geom *geo,
>  				int struct_version);
>  
>  #endif	/* __XFS_SB_H__ */
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index b6ae3597bfb0..c9ed5b6cb391 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -55,7 +55,7 @@ xfs_dir3_get_dtype(
>  	return xfs_dir3_filetype_table[filetype];
>  }
>  
> -STATIC int
> +STATIC void
>  xfs_dir2_sf_getdents(
>  	struct xfs_da_args	*args,
>  	struct dir_context	*ctx)
> @@ -80,7 +80,7 @@ xfs_dir2_sf_getdents(
>  	 * If the block number in the offset is out of range, we're done.
>  	 */
>  	if (xfs_dir2_dataptr_to_db(geo, ctx->pos) > geo->datablk)
> -		return 0;
> +		return;
>  
>  	/*
>  	 * Precalculate offsets for . and .. as we will always need them.
> @@ -99,7 +99,7 @@ xfs_dir2_sf_getdents(
>  	if (ctx->pos <= dot_offset) {
>  		ctx->pos = dot_offset & 0x7fffffff;
>  		if (!dir_emit(ctx, ".", 1, dp->i_ino, DT_DIR))
> -			return 0;
> +			return;
>  	}
>  
>  	/*
> @@ -109,7 +109,7 @@ xfs_dir2_sf_getdents(
>  		ino = dp->d_ops->sf_get_parent_ino(sfp);
>  		ctx->pos = dotdot_offset & 0x7fffffff;
>  		if (!dir_emit(ctx, "..", 2, ino, DT_DIR))
> -			return 0;
> +			return;
>  	}
>  
>  	/*
> @@ -132,13 +132,12 @@ xfs_dir2_sf_getdents(
>  		ctx->pos = off & 0x7fffffff;
>  		if (!dir_emit(ctx, (char *)sfep->name, sfep->namelen, ino,
>  			    xfs_dir3_get_dtype(dp->i_mount, filetype)))
> -			return 0;
> +			return;
>  		sfep = dp->d_ops->sf_nextentry(sfp, sfep);
>  	}
>  
>  	ctx->pos = xfs_dir2_db_off_to_dataptr(geo, geo->datablk + 1, 0) &
>  								0x7fffffff;
> -	return 0;
>  }
>  
>  /*
> @@ -513,7 +512,7 @@ xfs_readdir(
>  	size_t			bufsize)
>  {
>  	struct xfs_da_args	args = { NULL };
> -	int			rval;
> +	int			rval = 0;
>  	int			v;
>  
>  	trace_xfs_readdir(dp);
> @@ -529,7 +528,7 @@ xfs_readdir(
>  	args.trans = tp;
>  
>  	if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
> -		rval = xfs_dir2_sf_getdents(&args, ctx);
> +		xfs_dir2_sf_getdents(&args, ctx);
>  	else if ((rval = xfs_dir2_isblock(&args, &v)))
>  		;
>  	else if (v)
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 523792768080..2647a6ee7d79 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -687,7 +687,7 @@ xfs_growfs_log(
>   * exported through ioctl XFS_IOC_FSCOUNTS
>   */
>  
> -int
> +void
>  xfs_fs_counts(
>  	xfs_mount_t		*mp,
>  	xfs_fsop_counts_t	*cnt)
> @@ -700,7 +700,6 @@ xfs_fs_counts(
>  	spin_lock(&mp->m_sb_lock);
>  	cnt->freertx = mp->m_sb.sb_frextents;
>  	spin_unlock(&mp->m_sb_lock);
> -	return 0;
>  }
>  
>  /*
> diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
> index 20484ed5e919..2108f2191daf 100644
> --- a/fs/xfs/xfs_fsops.h
> +++ b/fs/xfs/xfs_fsops.h
> @@ -20,7 +20,7 @@
>  
>  extern int xfs_growfs_data(xfs_mount_t *mp, xfs_growfs_data_t *in);
>  extern int xfs_growfs_log(xfs_mount_t *mp, xfs_growfs_log_t *in);
> -extern int xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
> +extern void xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
>  extern int xfs_reserve_blocks(xfs_mount_t *mp, uint64_t *inval,
>  				xfs_fsop_resblks_t *outval);
>  extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 2b70c8b4cee2..3a9b193a7e8d 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1116,7 +1116,7 @@ xfs_droplink(
>  /*
>   * Increment the link count on an inode & log the change.
>   */
> -static int
> +static void 
>  xfs_bumplink(
>  	xfs_trans_t *tp,
>  	xfs_inode_t *ip)
> @@ -1126,7 +1126,6 @@ xfs_bumplink(
>  	ASSERT(ip->i_d.di_version > 1);
>  	inc_nlink(VFS_I(ip));
>  	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> -	return 0;
>  }
>  
>  int
> @@ -1238,10 +1237,7 @@ xfs_create(
>  		error = xfs_dir_init(tp, ip, dp);
>  		if (error)
>  			goto out_bmap_cancel;
> -
> -		error = xfs_bumplink(tp, dp);
> -		if (error)
> -			goto out_bmap_cancel;
> +		xfs_bumplink(tp, dp);
>  	}
>  
>  	/*
> @@ -1468,9 +1464,7 @@ xfs_link(
>  	xfs_trans_ichgtime(tp, tdp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
>  	xfs_trans_log_inode(tp, tdp, XFS_ILOG_CORE);
>  
> -	error = xfs_bumplink(tp, sip);
> -	if (error)
> -		goto error_return;
> +	xfs_bumplink(tp, sip);
>  
>  	/*
>  	 * If this is a synchronous mount, make sure that the
> @@ -2815,9 +2809,7 @@ xfs_cross_rename(
>  				error = xfs_droplink(tp, dp2);
>  				if (error)
>  					goto out_trans_abort;
> -				error = xfs_bumplink(tp, dp1);
> -				if (error)
> -					goto out_trans_abort;
> +				xfs_bumplink(tp, dp1);
>  			}
>  
>  			/*
> @@ -2842,9 +2834,7 @@ xfs_cross_rename(
>  				error = xfs_droplink(tp, dp1);
>  				if (error)
>  					goto out_trans_abort;
> -				error = xfs_bumplink(tp, dp2);
> -				if (error)
> -					goto out_trans_abort;
> +				xfs_bumplink(tp, dp2);
>  			}
>  
>  			/*
> @@ -3048,11 +3038,9 @@ xfs_rename(
>  		xfs_trans_ichgtime(tp, target_dp,
>  					XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
>  
> -		if (new_parent && src_is_directory) {
> -			error = xfs_bumplink(tp, target_dp);
> -			if (error)
> -				goto out_bmap_cancel;
> -		}
> +		if (new_parent && src_is_directory)
> +			xfs_bumplink(tp, target_dp);
> +
>  	} else { /* target_ip != NULL */
>  		/*
>  		 * If target exists and it's a directory, check that both
> @@ -3172,9 +3160,7 @@ xfs_rename(
>  	 */
>  	if (wip) {
>  		ASSERT(VFS_I(wip)->i_nlink == 0);
> -		error = xfs_bumplink(tp, wip);
> -		if (error)
> -			goto out_bmap_cancel;
> +		xfs_bumplink(tp, wip);
>  		error = xfs_iunlink_remove(tp, wip);
>  		if (error)
>  			goto out_bmap_cancel;
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 89fb1eb80aae..4ba8939145b8 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -808,11 +808,8 @@ xfs_ioc_fsgeometry_v1(
>  	void			__user *arg)
>  {
>  	xfs_fsop_geom_t         fsgeo;
> -	int			error;
>  
> -	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
> -	if (error)
> -		return error;
> +	xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
>  
>  	/*
>  	 * Caller should have passed an argument of type
> @@ -830,11 +827,8 @@ xfs_ioc_fsgeometry(
>  	void			__user *arg)
>  {
>  	xfs_fsop_geom_t		fsgeo;
> -	int			error;
>  
> -	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 4);
> -	if (error)
> -		return error;
> +	xfs_fs_geometry(&mp->m_sb, &fsgeo, 4);
>  
>  	if (copy_to_user(arg, &fsgeo, sizeof(fsgeo)))
>  		return -EFAULT;
> @@ -1963,10 +1957,7 @@ xfs_file_ioctl(
>  	case XFS_IOC_FSCOUNTS: {
>  		xfs_fsop_counts_t out;
>  
> -		error = xfs_fs_counts(mp, &out);
> -		if (error)
> -			return error;
> -
> +		xfs_fs_counts(mp, &out);
>  		if (copy_to_user(arg, &out, sizeof(out)))
>  			return -EFAULT;
>  		return 0;
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 10fbde359649..bb868ca31319 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -65,11 +65,9 @@ xfs_compat_ioc_fsgeometry_v1(
>  	compat_xfs_fsop_geom_v1_t __user *arg32)
>  {
>  	xfs_fsop_geom_t		  fsgeo;
> -	int			  error;
>  
> -	error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
> -	if (error)
> -		return error;
> +	xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
> +
>  	/* The 32-bit variant simply has some padding at the end */
>  	if (copy_to_user(arg32, &fsgeo, sizeof(struct compat_xfs_fsop_geom_v1)))
>  		return -EFAULT;
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 2fcd9ed5d075..65121e587e95 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -56,7 +56,7 @@ STATIC int
>  xlog_space_left(
>  	struct xlog		*log,
>  	atomic64_t		*head);
> -STATIC int
> +STATIC void
>  xlog_sync(
>  	struct xlog		*log,
>  	struct xlog_in_core	*iclog);
> @@ -1769,7 +1769,7 @@ xlog_cksum(
>   * ensure that completes before tearing down the iclogbufs. Hence we need to
>   * hold the buffer lock across the log IO to acheive that.
>   */
> -STATIC int
> +STATIC void
>  xlog_bdstrat(
>  	struct xfs_buf		*bp)
>  {
> @@ -1786,11 +1786,10 @@ xlog_bdstrat(
>  		 * doing it here. Similarly, IO completion will unlock the
>  		 * buffer, so we don't do it here.
>  		 */
> -		return 0;
> +		return;
>  	}
>  
>  	xfs_buf_submit(bp);
> -	return 0;
>  }
>  
>  /*
> @@ -1818,7 +1817,7 @@ xlog_bdstrat(
>   * is added immediately before calling bwrite().
>   */
>  
> -STATIC int
> +STATIC void
>  xlog_sync(
>  	struct xlog		*log,
>  	struct xlog_in_core	*iclog)
> @@ -1829,7 +1828,6 @@ xlog_sync(
>  	uint		count_init;	/* initial count before roundup */
>  	int		roundoff;       /* roundoff to BB or stripe */
>  	int		split = 0;	/* split write into two regions */
> -	int		error;
>  	int		v2 = xfs_sb_version_haslogv2(&log->l_mp->m_sb);
>  	int		size;
>  
> @@ -1948,11 +1946,8 @@ xlog_sync(
>  	 * Don't call xfs_bwrite here. We do log-syncs even when the filesystem
>  	 * is shutting down.
>  	 */
> -	error = xlog_bdstrat(bp);
> -	if (error) {
> -		xfs_buf_ioerror_alert(bp, "xlog_sync");
> -		return error;
> -	}
> +	xlog_bdstrat(bp);
> +
>  	if (split) {
>  		bp = iclog->ic_log->l_xbuf;
>  		XFS_BUF_SET_ADDR(bp, 0);	     /* logical 0 */
> @@ -1967,13 +1962,8 @@ xlog_sync(
>  
>  		/* account for internal log which doesn't start at block #0 */
>  		XFS_BUF_SET_ADDR(bp, XFS_BUF_ADDR(bp) + log->l_logBBstart);
> -		error = xlog_bdstrat(bp);
> -		if (error) {
> -			xfs_buf_ioerror_alert(bp, "xlog_sync (split)");
> -			return error;
> -		}
> +		xlog_bdstrat(bp);
>  	}
> -	return 0;
>  }	/* xlog_sync */
>  
>  /*
> @@ -3220,7 +3210,7 @@ xlog_state_release_iclog(
>  	 * flags after this point.
>  	 */
>  	if (sync)
> -		return xlog_sync(log, iclog);
> +		xlog_sync(log, iclog);
>  	return 0;
>  }	/* xlog_state_release_iclog */
>  
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 06a09cb948b5..a0f11bab3add 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2011,7 +2011,7 @@ xlog_recover_reorder_trans(
>   * record in the table to tell us how many times we expect to see this
>   * record during the second pass.
>   */
> -STATIC int
> +STATIC void
>  xlog_recover_buffer_pass1(
>  	struct xlog			*log,
>  	struct xlog_recover_item	*item)
> @@ -2025,7 +2025,7 @@ xlog_recover_buffer_pass1(
>  	 */
>  	if (!(buf_f->blf_flags & XFS_BLF_CANCEL)) {
>  		trace_xfs_log_recover_buf_not_cancel(log, buf_f);
> -		return 0;
> +		return;
>  	}
>  
>  	/*
> @@ -2038,7 +2038,7 @@ xlog_recover_buffer_pass1(
>  		    bcp->bc_len == buf_f->blf_len) {
>  			bcp->bc_refcount++;
>  			trace_xfs_log_recover_buf_cancel_ref_inc(log, buf_f);
> -			return 0;
> +			return;
>  		}
>  	}
>  
> @@ -2049,7 +2049,6 @@ xlog_recover_buffer_pass1(
>  	list_add_tail(&bcp->bc_list, bucket);
>  
>  	trace_xfs_log_recover_buf_cancel_add(log, buf_f);
> -	return 0;
>  }
>  
>  /*
> @@ -3270,7 +3269,7 @@ xlog_recover_inode_pass2(
>   * structure, so that we know not to do any dquot item or dquot buffer recovery,
>   * of that type.
>   */
> -STATIC int
> +STATIC void
>  xlog_recover_quotaoff_pass1(
>  	struct xlog			*log,
>  	struct xlog_recover_item	*item)
> @@ -3288,8 +3287,6 @@ xlog_recover_quotaoff_pass1(
>  		log->l_quotaoffs_flag |= XFS_DQ_PROJ;
>  	if (qoff_f->qf_flags & XFS_GQUOTA_ACCT)
>  		log->l_quotaoffs_flag |= XFS_DQ_GROUP;
> -
> -	return 0;
>  }
>  
>  /*
> @@ -3449,7 +3446,7 @@ xlog_recover_efi_pass2(
>   * equal to that in the EFD format structure. If we find it we drop the EFD
>   * reference, which removes the EFI from the AIL and frees it.
>   */
> -STATIC int
> +STATIC void
>  xlog_recover_efd_pass2(
>  	struct xlog			*log,
>  	struct xlog_recover_item	*item)
> @@ -3493,8 +3490,6 @@ xlog_recover_efd_pass2(
>  
>  	xfs_trans_ail_cursor_done(&cur);
>  	spin_unlock(&ailp->ail_lock);
> -
> -	return 0;
>  }
>  
>  /*
> @@ -3545,7 +3540,7 @@ xlog_recover_rui_pass2(
>   * equal to that in the RUD format structure. If we find it we drop the RUD
>   * reference, which removes the RUI from the AIL and frees it.
>   */
> -STATIC int
> +STATIC void
>  xlog_recover_rud_pass2(
>  	struct xlog			*log,
>  	struct xlog_recover_item	*item)
> @@ -3586,8 +3581,6 @@ xlog_recover_rud_pass2(
>  
>  	xfs_trans_ail_cursor_done(&cur);
>  	spin_unlock(&ailp->ail_lock);
> -
> -	return 0;
>  }
>  
>  /*
> @@ -4072,9 +4065,11 @@ xlog_recover_commit_pass1(
>  
>  	switch (ITEM_TYPE(item)) {
>  	case XFS_LI_BUF:
> -		return xlog_recover_buffer_pass1(log, item);
> +		xlog_recover_buffer_pass1(log, item);
> +		return 0;
>  	case XFS_LI_QUOTAOFF:
> -		return xlog_recover_quotaoff_pass1(log, item);
> +		xlog_recover_quotaoff_pass1(log, item);
> +		return 0;
>  	case XFS_LI_INODE:
>  	case XFS_LI_EFI:
>  	case XFS_LI_EFD:
> @@ -4115,11 +4110,13 @@ xlog_recover_commit_pass2(
>  	case XFS_LI_EFI:
>  		return xlog_recover_efi_pass2(log, item, trans->r_lsn);
>  	case XFS_LI_EFD:
> -		return xlog_recover_efd_pass2(log, item);
> +		xlog_recover_efd_pass2(log, item);
> +		return 0;
>  	case XFS_LI_RUI:
>  		return xlog_recover_rui_pass2(log, item, trans->r_lsn);
>  	case XFS_LI_RUD:
> -		return xlog_recover_rud_pass2(log, item);
> +		xlog_recover_rud_pass2(log, item);
> +		return 0;
>  	case XFS_LI_CUI:
>  		return xlog_recover_cui_pass2(log, item, trans->r_lsn);
>  	case XFS_LI_CUD:
> @@ -5162,7 +5159,7 @@ xlog_recover_process_iunlinks(
>  	}
>  }
>  
> -STATIC int
> +STATIC void
>  xlog_unpack_data(
>  	struct xlog_rec_header	*rhead,
>  	char			*dp,
> @@ -5185,8 +5182,6 @@ xlog_unpack_data(
>  			dp += BBSIZE;
>  		}
>  	}
> -
> -	return 0;
>  }
>  
>  /*
> @@ -5201,11 +5196,9 @@ xlog_recover_process(
>  	int			pass,
>  	struct list_head	*buffer_list)
>  {
> -	int			error;
>  	__le32			old_crc = rhead->h_crc;
>  	__le32			crc;
>  
> -
>  	crc = xlog_cksum(log, rhead, dp, be32_to_cpu(rhead->h_len));
>  
>  	/*
> @@ -5244,9 +5237,7 @@ xlog_recover_process(
>  			return -EFSCORRUPTED;
>  	}
>  
> -	error = xlog_unpack_data(rhead, dp, log);
> -	if (error)
> -		return error;
> +	xlog_unpack_data(rhead, dp, log);
>  
>  	return xlog_recover_process_data(log, rhash, rhead, dp, pass,
>  					 buffer_list);
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index d71424052917..efb6c1d5f0bc 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -475,7 +475,7 @@ struct proc_xfs_info {
>  	char		*str;
>  };
>  
> -STATIC int
> +STATIC void
>  xfs_showargs(
>  	struct xfs_mount	*mp,
>  	struct seq_file		*m)
> @@ -555,8 +555,6 @@ xfs_showargs(
>  
>  	if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
>  		seq_puts(m, ",noquota");
> -
> -	return 0;
>  }
>  static uint64_t
>  xfs_max_file_offset(
> @@ -1462,7 +1460,8 @@ xfs_fs_show_options(
>  	struct seq_file		*m,
>  	struct dentry		*root)
>  {
> -	return xfs_showargs(XFS_M(root->d_sb), m);
> +	xfs_showargs(XFS_M(root->d_sb), m);
> +	return 0;
>  }
>  
>  /*
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/5] xfs: make several functions void

[Eric Sandeen]

On 5/4/18 5:29 PM, Darrick J. Wong wrote:
>> Subject: [PATCH 6/5] xfs: make several functions void
> 
> Usual patch cramming practice is to make this a reply to the cover
> letter, not 5/5.

Yup I noticed that when it hit my inbox, my bad.

> Also, "make several functions return void"?

sure.  "making them void" could be interpreted as vulgar.  ;)

...

>> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
>> index 2135b8e67dcc..73f2c1ae2dcf 100644
>> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
>> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
>> @@ -59,7 +59,7 @@
>>   */
>>  STATIC int xfs_attr3_leaf_create(struct xfs_da_args *args,
>>  				 xfs_dablk_t which_block, struct xfs_buf **bpp);
>> -STATIC int xfs_attr3_leaf_add_work(struct xfs_buf *leaf_buffer,
>> +STATIC void xfs_attr3_leaf_add_work(struct xfs_buf *leaf_buffer,
> 
> /me hates to be a PITA, but some of these functions ASSERT on on-disk
> metadata checks that fail.  Sometimes the checks are for things that are
> already caught by the verifiers & so therefore ensure we haven't
> corrupted in-core state, but others could very well be things that
> should be turned into -EFSCORRUPTED return.
> 
> How about splitting these up and only do a few functions (or a single
> file) at a time?  (And ideally after I land the series that starts the
> conversion of ASSERT-on-disk-corruption to EFSCORRUPTED-return that's
> also out for review...)

Sure, fine by me, or maybe I'll just hit all the bog-simple ones and leave
the rest, for now.

Thanks,
-Eric

Re: [PATCH 5/5] xfs: print specific dqblk that failed verifiers

[Darrick J. Wong]

On Fri, May 04, 2018 at 12:35:40PM -0500, Eric Sandeen wrote:
> Rather than printing the top of the buffer that held a corrupted dqblk,
> restructure things to print out the specific one that failed by pushing
> the calls to the verifier_error function down into the verifier which
> iterates over the buffer and detects the error.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_dquot_buf.c | 41 +++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
> index d71c0b53536b..fb23d941e799 100644
> --- a/fs/xfs/libxfs/xfs_dquot_buf.c
> +++ b/fs/xfs/libxfs/xfs_dquot_buf.c
> @@ -152,7 +152,8 @@ xfs_dqblk_repair(
>  STATIC bool
>  xfs_dquot_buf_verify_crc(
>  	struct xfs_mount	*mp,
> -	struct xfs_buf		*bp)
> +	struct xfs_buf		*bp,
> +	bool			readahead)
>  {
>  	struct xfs_dqblk	*d = (struct xfs_dqblk *)bp->b_addr;
>  	int			ndquots;
> @@ -173,8 +174,12 @@ xfs_dquot_buf_verify_crc(
>  
>  	for (i = 0; i < ndquots; i++, d++) {
>  		if (!xfs_verify_cksum((char *)d, sizeof(struct xfs_dqblk),
> -				 XFS_DQUOT_CRC_OFF))
> +				 XFS_DQUOT_CRC_OFF)) {
> +			if (!readahead)
> +				xfs_buf_verifier_error(bp, -EFSBADCRC, __func__,
> +					d, sizeof(*d), __this_address);
>  			return false;
> +		}
>  	}
>  	return true;
>  }
> @@ -182,7 +187,8 @@ xfs_dquot_buf_verify_crc(
>  STATIC xfs_failaddr_t
>  xfs_dquot_buf_verify(
>  	struct xfs_mount	*mp,
> -	struct xfs_buf		*bp)
> +	struct xfs_buf		*bp,
> +	bool			readahead)
>  {
>  	struct xfs_dqblk	*dqb = (struct xfs_dqblk *)bp->b_addr;
>  	xfs_failaddr_t		fa;
> @@ -216,8 +222,13 @@ xfs_dquot_buf_verify(
>  			id = be32_to_cpu(ddq->d_id);
>  
>  		fa = xfs_dqblk_verify(mp, &dqb[i], id + i, 0);
> -		if (fa)
> +		if (fa) {
> +			if (!readahead)
> +				xfs_buf_verifier_error(bp, -EFSCORRUPTED,
> +					__func__, &dqb[i],
> +					sizeof(struct xfs_dqblk), fa);
>  			return fa;
> +		}
>  	}
>  
>  	return NULL;
> @@ -229,7 +240,7 @@ xfs_dquot_buf_verify_struct(
>  {
>  	struct xfs_mount	*mp = bp->b_target->bt_mount;
>  
> -	return xfs_dquot_buf_verify(mp, bp);
> +	return xfs_dquot_buf_verify(mp, bp, false);
>  }
>  
>  static void
> @@ -237,15 +248,10 @@ xfs_dquot_buf_read_verify(
>  	struct xfs_buf		*bp)
>  {
>  	struct xfs_mount	*mp = bp->b_target->bt_mount;
> -	xfs_failaddr_t		fa;
>  
> -	if (!xfs_dquot_buf_verify_crc(mp, bp))
> -		xfs_verifier_error(bp, -EFSBADCRC, __this_address);
> -	else {
> -		fa = xfs_dquot_buf_verify(mp, bp);
> -		if (fa)
> -			xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
> -	}
> +	if (!xfs_dquot_buf_verify_crc(mp, bp, false))
> +		return;
> +	xfs_dquot_buf_verify(mp, bp, false);
>  }
>  
>  /*
> @@ -260,8 +266,8 @@ xfs_dquot_buf_readahead_verify(
>  {
>  	struct xfs_mount	*mp = bp->b_target->bt_mount;
>  
> -	if (!xfs_dquot_buf_verify_crc(mp, bp) ||
> -	    xfs_dquot_buf_verify(mp, bp) != NULL) {
> +	if (!xfs_dquot_buf_verify_crc(mp, bp, true) ||
> +	    xfs_dquot_buf_verify(mp, bp, true) != NULL) {
>  		xfs_buf_ioerror(bp, -EIO);
>  		bp->b_flags &= ~XBF_DONE;
>  	}
> @@ -277,11 +283,8 @@ xfs_dquot_buf_write_verify(
>  	struct xfs_buf		*bp)
>  {
>  	struct xfs_mount	*mp = bp->b_target->bt_mount;
> -	xfs_failaddr_t		fa;
>  
> -	fa = xfs_dquot_buf_verify(mp, bp);
> -	if (fa)
> -		xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
> +	xfs_dquot_buf_verify(mp, bp, false);
>  }
>  
>  const struct xfs_buf_ops xfs_dquot_buf_ops = {
> -- 
> 2.17.0
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] xfs: print specific dqblk that failed verifiers

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>