Skip to contentSkip to conversation

xfs: Remove XFS_MOUNT_RETERR

PATCH Superseded Jeff Liu 2 participants 3 messages May 1, 2013 – May 2, 2013 XFS filesystem

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jeff Liu

author
#0 lore raw

PATCH 1/2 xfs: Remove XFS_MOUNT_RETERR

From: Jie Liu <[email protected]>

XFS_MOUNT_RETERR is going to be set at xfs_parseargs() if mp->m_dalign is enabled, so any time we enter "if (mp->m_dalign)" branch in xfs_update_alignment(), XFS_MOUNT_RETERR is set and so we always be emitting a warning and returning an error.

Hence, we can remove it and get rid of a couple of redundant check up against it at xfs_upate_alignment().

Thanks Dave Chinner for the confirmation.

Cc Dave Chinner <[email protected]>
Cc Mark Tinguely <[email protected]>
---
 fs/xfs/xfs_mount.c |   39 ++++++++++++---------------------------
 fs/xfs/xfs_mount.h |    2 --
 fs/xfs/xfs_super.c |    5 +----
 3 files changed, 13 insertions(+), 33 deletions(-)
fs/xfs/xfs_mount.c +12 -27
index 3806088..29e8de8 100644
@@ -872,42 +872,27 @@ xfs_update_alignment(xfs_mount_t *mp)
 		 */
 		if ((BBTOB(mp->m_dalign) & mp->m_blockmask) ||
 		    (BBTOB(mp->m_swidth) & mp->m_blockmask)) {
-			if (mp->m_flags & XFS_MOUNT_RETERR) {
-				xfs_warn(mp, "alignment check failed: "
-					 "(sunit/swidth vs. blocksize)");
-				return XFS_ERROR(EINVAL);
-			}
-			mp->m_dalign = mp->m_swidth = 0;
+			xfs_warn(mp, "alignment check failed: "
+				 "(sunit/swidth vs. blocksize)");
+			return XFS_ERROR(EINVAL);
 		} else {
 			/*
 			 * Convert the stripe unit and width to FSBs.
 			 */
 			mp->m_dalign = XFS_BB_TO_FSBT(mp, mp->m_dalign);
 			if (mp->m_dalign && (sbp->sb_agblocks % mp->m_dalign)) {
-				if (mp->m_flags & XFS_MOUNT_RETERR) {
-					xfs_warn(mp, "alignment check failed: "
-						 "(sunit/swidth vs. ag size)");
-					return XFS_ERROR(EINVAL);
-				}
-				xfs_warn(mp,
-		"stripe alignment turned off: sunit(%d)/swidth(%d) "
-		"incompatible with agsize(%d)",
-					mp->m_dalign, mp->m_swidth,
-					sbp->sb_agblocks);
-
-				mp->m_dalign = 0;
-				mp->m_swidth = 0;
+				xfs_warn(mp, "alignment check failed: "
+			"sunit(%d)/swidth(%d) incompatible with agsize(%d)",
+					 mp->m_dalign, mp->m_swidth,
+					 sbp->sb_agblocks);
+				return XFS_ERROR(EINVAL);
 			} else if (mp->m_dalign) {
 				mp->m_swidth = XFS_BB_TO_FSBT(mp, mp->m_swidth);
 			} else {
-				if (mp->m_flags & XFS_MOUNT_RETERR) {
-					xfs_warn(mp, "alignment check failed: "
-						"sunit(%d) less than bsize(%d)",
-						mp->m_dalign,
-						mp->m_blockmask +1);
-					return XFS_ERROR(EINVAL);
-				}
-				mp->m_swidth = 0;
+				xfs_warn(mp, "alignment check failed: "
+					 "sunit(%d) less than bsize(%d)",
+					 mp->m_dalign, mp->m_blockmask + 1);
+				return XFS_ERROR(EINVAL);
 			}
 		}
 
fs/xfs/xfs_mount.h +0 -2
index bc90706..8145412 100644
@@ -230,8 +230,6 @@ typedef struct xfs_mount {
 						   operations, typically for
 						   disk errors in metadata */
 #define XFS_MOUNT_DISCARD	(1ULL << 5)	/* discard unused blocks */
-#define XFS_MOUNT_RETERR	(1ULL << 6)     /* return alignment errors to
-						   user */
 #define XFS_MOUNT_NOALIGN	(1ULL << 7)	/* turn off stripe alignment
 						   allocations */
 #define XFS_MOUNT_ATTR2		(1ULL << 8)	/* allow use of attr2 format */
fs/xfs/xfs_super.c +1 -4
index ea341ce..51da4d2 100644
@@ -446,11 +446,8 @@ done:
 		 * Before the mount call ends we will convert
 		 * these to FSBs.
 		 */
-		if (dsunit) {
+		if (dsunit)
 			mp->m_dalign = dsunit;
-			mp->m_flags |= XFS_MOUNT_RETERR;
-		}
-
 		if (dswidth)
 			mp->m_swidth = dswidth;
 	}
Signature · 6 lines
1.7.9.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#1 lore raw
Quoted 27 lines · Jeff Liu

From: Jie Liu <[email protected]>

XFS_MOUNT_RETERR is going to be set at xfs_parseargs() if mp->m_dalign is enabled, so any time we enter "if (mp->m_dalign)" branch in xfs_update_alignment(), XFS_MOUNT_RETERR is set and so we always be emitting a warning and returning an error.

Hence, we can remove it and get rid of a couple of redundant check up against it at xfs_upate_alignment().

Thanks Dave Chinner for the confirmation.

Cc Dave Chinner <[email protected]>
Cc Mark Tinguely <[email protected]>
---
 fs/xfs/xfs_mount.c |   39 ++++++++++++---------------------------
 fs/xfs/xfs_mount.h |    2 --
 fs/xfs/xfs_super.c |    5 +----
 3 files changed, 13 insertions(+), 33 deletions(-)
fs/xfs/xfs_mount.c +0 -0
index 3806088..29e8de8 100644
@@ -872,42 +872,27 @@ xfs_update_alignment(xfs_mount_t *mp)
 		 */

fs/xfs/xfs_mount.c xfs_update_alignment(xfs_mount_t *mp) · line 876

Quoted 10 lines
fs/xfs/xfs_mount.c +2 -6
 		if ((BBTOB(mp->m_dalign) & mp->m_blockmask) ||
 		    (BBTOB(mp->m_swidth) & mp->m_blockmask)) {
-			if (mp->m_flags & XFS_MOUNT_RETERR) {
-				xfs_warn(mp, "alignment check failed: "
-					 "(sunit/swidth vs. blocksize)");
-				return XFS_ERROR(EINVAL);
-			}
-			mp->m_dalign = mp->m_swidth = 0;
+			xfs_warn(mp, "alignment check failed: "
+				 "(sunit/swidth vs. blocksize)");

Let's convert these format strings to a single line at the same time and be consistent with output. ie:

			xfs_warn(mp,
	"alignment check failed: sunit/swidth vs. blocksize(%d)",
				sbp->sb_blocksize);
Quoted 12 lines
fs/xfs/xfs_mount.c +1 -5
+			return XFS_ERROR(EINVAL);
 		} else {
 			/*
 			 * Convert the stripe unit and width to FSBs.
 			 */
 			mp->m_dalign = XFS_BB_TO_FSBT(mp, mp->m_dalign);
 			if (mp->m_dalign && (sbp->sb_agblocks % mp->m_dalign)) {
-				if (mp->m_flags & XFS_MOUNT_RETERR) {
-					xfs_warn(mp, "alignment check failed: "
-						 "(sunit/swidth vs. ag size)");
-					return XFS_ERROR(EINVAL);
-				}

fs/xfs/xfs_mount.c xfs_update_alignment(xfs_mount_t *mp) · line 885

Quoted 10 lines
fs/xfs/xfs_mount.c +2 -8
-				xfs_warn(mp,
-		"stripe alignment turned off: sunit(%d)/swidth(%d) "
-		"incompatible with agsize(%d)",
-					mp->m_dalign, mp->m_swidth,
-					sbp->sb_agblocks);
-
-				mp->m_dalign = 0;
-				mp->m_swidth = 0;
+				xfs_warn(mp, "alignment check failed: "
+			"sunit(%d)/swidth(%d) incompatible with agsize(%d)",
	"alignment check failed: sunit/swidth vs. agsize(%d)",
					sbp->sb_agblocks);
Quoted 7 lines
fs/xfs/xfs_mount.c +3 -1
+					 mp->m_dalign, mp->m_swidth,
+					 sbp->sb_agblocks);
+				return XFS_ERROR(EINVAL);
 			} else if (mp->m_dalign) {
 				mp->m_swidth = XFS_BB_TO_FSBT(mp, mp->m_swidth);
 			} else {
-				if (mp->m_flags & XFS_MOUNT_RETERR) {

fs/xfs/xfs_mount.c xfs_update_alignment(xfs_mount_t *mp) · line 894

Quoted 10 lines
fs/xfs/xfs_mount.c +3 -7
-					xfs_warn(mp, "alignment check failed: "
-						"sunit(%d) less than bsize(%d)",
-						mp->m_dalign,
-						mp->m_blockmask +1);
-					return XFS_ERROR(EINVAL);
-				}
-				mp->m_swidth = 0;
+				xfs_warn(mp, "alignment check failed: "
+					 "sunit(%d) less than bsize(%d)",
+					 mp->m_dalign, mp->m_blockmask + 1);
	"alignment check failed: sunit(%d) less than bsize(%d)",
					mp->m_dalign, sbp->sb_blocksize);
Quoted 23 lines
fs/xfs/xfs_mount.c +1 -0
+				return XFS_ERROR(EINVAL);
 			}
 		}
 
fs/xfs/xfs_mount.h +0 -2
index bc90706..8145412 100644
@@ -230,8 +230,6 @@ typedef struct xfs_mount {
 						   operations, typically for
 						   disk errors in metadata */
 #define XFS_MOUNT_DISCARD	(1ULL << 5)	/* discard unused blocks */
-#define XFS_MOUNT_RETERR	(1ULL << 6)     /* return alignment errors to
-						   user */
 #define XFS_MOUNT_NOALIGN	(1ULL << 7)	/* turn off stripe alignment
 						   allocations */
 #define XFS_MOUNT_ATTR2		(1ULL << 8)	/* allow use of attr2 format */
fs/xfs/xfs_super.c +0 -0
index ea341ce..51da4d2 100644
@@ -446,11 +446,8 @@ done:
 		 * Before the mount call ends we will convert

fs/xfs/xfs_super.c done: · line 452

Quoted 10 lines
fs/xfs/xfs_super.c +1 -4
 		 * these to FSBs.
 		 */
-		if (dsunit) {
+		if (dsunit)
 			mp->m_dalign = dsunit;
-			mp->m_flags |= XFS_MOUNT_RETERR;
-		}
-
 		if (dswidth)
 			mp->m_swidth = dswidth;

This code can be simplified, too. We've already done this check:

	if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
                xfs_warn(mp, "sunit and swidth must be specified together");
                return EINVAL;
        }

So we know that dsunit/dswidth are either both zero or both set, so the above code could simply end up as:

        if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
                /*
                 * At this point the superblock has not been read
                 * in, therefore we do not know the block size.
                 * Before the mount call ends we will convert
                 * these to FSBs.
                 */
		mp->m_dalign = dsunit;
		mp->m_swidth = dswidth;
        }

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#2 lore raw
Quoted 42 lines · Dave Chinner
Quoted 27 lines · Jeff Liu

From: Jie Liu <[email protected]>

XFS_MOUNT_RETERR is going to be set at xfs_parseargs() if mp->m_dalign is enabled, so any time we enter "if (mp->m_dalign)" branch in xfs_update_alignment(), XFS_MOUNT_RETERR is set and so we always be emitting a warning and returning an error.

Hence, we can remove it and get rid of a couple of redundant check up against it at xfs_upate_alignment().

Thanks Dave Chinner for the confirmation.

Cc Dave Chinner <[email protected]>
Cc Mark Tinguely <[email protected]>
---
 fs/xfs/xfs_mount.c |   39 ++++++++++++---------------------------
 fs/xfs/xfs_mount.h |    2 --
 fs/xfs/xfs_super.c |    5 +----
 3 files changed, 13 insertions(+), 33 deletions(-)
fs/xfs/xfs_mount.c +0 -0
index 3806088..29e8de8 100644
@@ -872,42 +872,27 @@ xfs_update_alignment(xfs_mount_t *mp)
 		 */

fs/xfs/xfs_mount.c xfs_update_alignment(xfs_mount_t *mp) · line 876

Quoted 10 lines
fs/xfs/xfs_mount.c +2 -6
 		if ((BBTOB(mp->m_dalign) & mp->m_blockmask) ||
 		    (BBTOB(mp->m_swidth) & mp->m_blockmask)) {
-			if (mp->m_flags & XFS_MOUNT_RETERR) {
-				xfs_warn(mp, "alignment check failed: "
-					 "(sunit/swidth vs. blocksize)");
-				return XFS_ERROR(EINVAL);
-			}
-			mp->m_dalign = mp->m_swidth = 0;
+			xfs_warn(mp, "alignment check failed: "
+				 "(sunit/swidth vs. blocksize)");

Let's convert these format strings to a single line at the same time and be consistent with output. ie:

Quoted 3 lines
			xfs_warn(mp,
	"alignment check failed: sunit/swidth vs. blocksize(%d)",
				sbp->sb_blocksize);

Ok.

Quoted 91 lines
Quoted 13 lines
fs/xfs/xfs_mount.c +1 -5
+			return XFS_ERROR(EINVAL);
 		} else {
 			/*
 			 * Convert the stripe unit and width to FSBs.
 			 */
 			mp->m_dalign = XFS_BB_TO_FSBT(mp, mp->m_dalign);
 			if (mp->m_dalign && (sbp->sb_agblocks % mp->m_dalign)) {
-				if (mp->m_flags & XFS_MOUNT_RETERR) {
-					xfs_warn(mp, "alignment check failed: "
-						 "(sunit/swidth vs. ag size)");
-					return XFS_ERROR(EINVAL);
-				}

fs/xfs/xfs_mount.c xfs_update_alignment(xfs_mount_t *mp) · line 885

Quoted 10 lines
fs/xfs/xfs_mount.c +2 -8
-				xfs_warn(mp,
-		"stripe alignment turned off: sunit(%d)/swidth(%d) "
-		"incompatible with agsize(%d)",
-					mp->m_dalign, mp->m_swidth,
-					sbp->sb_agblocks);
-
-				mp->m_dalign = 0;
-				mp->m_swidth = 0;
+				xfs_warn(mp, "alignment check failed: "
+			"sunit(%d)/swidth(%d) incompatible with agsize(%d)",
	"alignment check failed: sunit/swidth vs. agsize(%d)",
					sbp->sb_agblocks);
Quoted 7 lines
fs/xfs/xfs_mount.c +3 -1
+					 mp->m_dalign, mp->m_swidth,
+					 sbp->sb_agblocks);
+				return XFS_ERROR(EINVAL);
 			} else if (mp->m_dalign) {
 				mp->m_swidth = XFS_BB_TO_FSBT(mp, mp->m_swidth);
 			} else {
-				if (mp->m_flags & XFS_MOUNT_RETERR) {

fs/xfs/xfs_mount.c xfs_update_alignment(xfs_mount_t *mp) · line 894

Quoted 10 lines
fs/xfs/xfs_mount.c +3 -7
-					xfs_warn(mp, "alignment check failed: "
-						"sunit(%d) less than bsize(%d)",
-						mp->m_dalign,
-						mp->m_blockmask +1);
-					return XFS_ERROR(EINVAL);
-				}
-				mp->m_swidth = 0;
+				xfs_warn(mp, "alignment check failed: "
+					 "sunit(%d) less than bsize(%d)",
+					 mp->m_dalign, mp->m_blockmask + 1);
	"alignment check failed: sunit(%d) less than bsize(%d)",
					mp->m_dalign, sbp->sb_blocksize);
Quoted 23 lines
fs/xfs/xfs_mount.c +1 -0
+				return XFS_ERROR(EINVAL);
 			}
 		}
 
fs/xfs/xfs_mount.h +0 -2
index bc90706..8145412 100644
@@ -230,8 +230,6 @@ typedef struct xfs_mount {
 						   operations, typically for
 						   disk errors in metadata */
 #define XFS_MOUNT_DISCARD	(1ULL << 5)	/* discard unused blocks */
-#define XFS_MOUNT_RETERR	(1ULL << 6)     /* return alignment errors to
-						   user */
 #define XFS_MOUNT_NOALIGN	(1ULL << 7)	/* turn off stripe alignment
 						   allocations */
 #define XFS_MOUNT_ATTR2		(1ULL << 8)	/* allow use of attr2 format */
fs/xfs/xfs_super.c +0 -0
index ea341ce..51da4d2 100644
@@ -446,11 +446,8 @@ done:
 		 * Before the mount call ends we will convert

fs/xfs/xfs_super.c done: · line 452

Quoted 10 lines
fs/xfs/xfs_super.c +1 -4
 		 * these to FSBs.
 		 */
-		if (dsunit) {
+		if (dsunit)
 			mp->m_dalign = dsunit;
-			mp->m_flags |= XFS_MOUNT_RETERR;
-		}
-
 		if (dswidth)
 			mp->m_swidth = dswidth;

This code can be simplified, too. We've already done this check:

	if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
                xfs_warn(mp, "sunit and swidth must be specified together");
                return EINVAL;
        }

So we know that dsunit/dswidth are either both zero or both set, so the above code could simply end up as:

Quoted 10 lines
        if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
                /*
                 * At this point the superblock has not been read
                 * in, therefore we do not know the block size.
                 * Before the mount call ends we will convert
                 * these to FSBs.
                 */
		mp->m_dalign = dsunit;
		mp->m_swidth = dswidth;
        }

Exactly, will take care of this in next round of post.

Thanks,
-Jeff

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs