public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] xfs: Remove XFS_MOUNT_RETERR
@ 2013-05-02 11:27 Jeff Liu
  2013-05-02 15:27 ` Mark Tinguely
  2013-06-19 19:55 ` Ben Myers
  0 siblings, 2 replies; 4+ messages in thread
From: Jeff Liu @ 2013-05-02 11:27 UTC (permalink / raw)
  To: xfs@oss.sgi.com; +Cc: Mark Tinguely

From: Jie Liu <jeff.liu@oracle.com>

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 suggestions of simplify the code
in xfs_parseargs().

Signed-off-by: Jie Liu <jeff.liu@oracle.com>
Cc: Dave Chinner <dchinner@redhat.com>
Cc: Mark Tinguely <tinguely@sgi.com>
---
 fs/xfs/xfs_mount.c |   37 +++++++++++--------------------------
 fs/xfs/xfs_mount.h |    2 --
 fs/xfs/xfs_super.c |   11 +++--------
 3 files changed, 14 insertions(+), 36 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 3806088..95bd4b5 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -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(%d)",
+				sbp->sb_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;
+			"alignment check failed: sunit/swidth vs. agsize(%d)",
+					 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, sbp->sb_blocksize);
+				return XFS_ERROR(EINVAL);
 			}
 		}
 
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index b68e56b..ace092a7 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -228,8 +228,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 */
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index ea341ce..7bf1389 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -439,20 +439,15 @@ xfs_parseargs(
 	}
 
 done:
-	if (!(mp->m_flags & XFS_MOUNT_NOALIGN)) {
+	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.
 		 */
-		if (dsunit) {
-			mp->m_dalign = dsunit;
-			mp->m_flags |= XFS_MOUNT_RETERR;
-		}
-
-		if (dswidth)
-			mp->m_swidth = dswidth;
+		mp->m_dalign = dsunit;
+		mp->m_swidth = dswidth;
 	}
 
 	if (mp->m_logbufs != -1 &&
-- 
1.7.9.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] xfs: Remove XFS_MOUNT_RETERR
  2013-05-02 11:27 [PATCH v3 1/2] xfs: Remove XFS_MOUNT_RETERR Jeff Liu
@ 2013-05-02 15:27 ` Mark Tinguely
  2013-05-02 15:34   ` Jeff Liu
  2013-06-19 19:55 ` Ben Myers
  1 sibling, 1 reply; 4+ messages in thread
From: Mark Tinguely @ 2013-05-02 15:27 UTC (permalink / raw)
  To: Jeff Liu; +Cc: xfs@oss.sgi.com

On 05/02/13 06:27, Jeff Liu wrote:
> From: Jie Liu<jeff.liu@oracle.com>
>
> 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 suggestions of simplify the code
> in xfs_parseargs().
>
> Signed-off-by: Jie Liu<jeff.liu@oracle.com>

"alignment check failed: sunit/swidth vs. blocksize(%d)",
				                   ^^^^
"alignment check failed: sunit/swidth vs. agsize(%d)",
                                                 ^^^^
"alignment check failed: sunit(%d) less than bsize(%d)",
                               ^^^^                ^^^^
I know you copied these statements verbatim, but I wonder why all
these statements do not have a space between the type and value?

Looks good.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] xfs: Remove XFS_MOUNT_RETERR
  2013-05-02 15:27 ` Mark Tinguely
@ 2013-05-02 15:34   ` Jeff Liu
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff Liu @ 2013-05-02 15:34 UTC (permalink / raw)
  To: Mark Tinguely; +Cc: xfs@oss.sgi.com

On 05/02/2013 11:27 PM, Mark Tinguely wrote:
> On 05/02/13 06:27, Jeff Liu wrote:
>> From: Jie Liu<jeff.liu@oracle.com>
>>
>> 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 suggestions of simplify the code
>> in xfs_parseargs().
>>
>> Signed-off-by: Jie Liu<jeff.liu@oracle.com>
> 
> "alignment check failed: sunit/swidth vs. blocksize(%d)",
> 				                   ^^^^
> "alignment check failed: sunit/swidth vs. agsize(%d)",
>                                                  ^^^^
> "alignment check failed: sunit(%d) less than bsize(%d)",
>                                ^^^^                ^^^^
> I know you copied these statements verbatim, but I wonder why all
> these statements do not have a space between the type and value?
Looks the old statements violate the above rule(don't have a space
between them) as there are many places formating the statements
with a space.

Thanks,
-Jeff
> 
> Looks good.
> 
> Reviewed-by: Mark Tinguely <tinguely@sgi.com>
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] xfs: Remove XFS_MOUNT_RETERR
  2013-05-02 11:27 [PATCH v3 1/2] xfs: Remove XFS_MOUNT_RETERR Jeff Liu
  2013-05-02 15:27 ` Mark Tinguely
@ 2013-06-19 19:55 ` Ben Myers
  1 sibling, 0 replies; 4+ messages in thread
From: Ben Myers @ 2013-06-19 19:55 UTC (permalink / raw)
  To: Jeff Liu; +Cc: Mark Tinguely, xfs@oss.sgi.com

On Thu, May 02, 2013 at 07:27:47PM +0800, Jeff Liu wrote:
> From: Jie Liu <jeff.liu@oracle.com>
> 
> 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 suggestions of simplify the code
> in xfs_parseargs().
> 
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> Cc: Dave Chinner <dchinner@redhat.com>
> Cc: Mark Tinguely <tinguely@sgi.com>

Applied.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-06-19 19:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-02 11:27 [PATCH v3 1/2] xfs: Remove XFS_MOUNT_RETERR Jeff Liu
2013-05-02 15:27 ` Mark Tinguely
2013-05-02 15:34   ` Jeff Liu
2013-06-19 19:55 ` Ben Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox