* [PATCH 1/2] xfs: Remove XFS_MOUNT_RETERR @ 2013-05-01 14:25 Jeff Liu 2013-05-02 1:12 ` Dave Chinner 0 siblings, 1 reply; 3+ messages in thread From: Jeff Liu @ 2013-05-01 14:25 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 confirmation. 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 | 39 ++++++++++++--------------------------- fs/xfs/xfs_mount.h | 2 -- fs/xfs/xfs_super.c | 5 +---- 3 files changed, 13 insertions(+), 33 deletions(-) diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index 3806088..29e8de8 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)"); + 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); } } diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h index bc90706..8145412 100644 --- a/fs/xfs/xfs_mount.h +++ b/fs/xfs/xfs_mount.h @@ -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 */ diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index ea341ce..51da4d2 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -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; } -- 1.7.9.5 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] xfs: Remove XFS_MOUNT_RETERR 2013-05-01 14:25 [PATCH 1/2] xfs: Remove XFS_MOUNT_RETERR Jeff Liu @ 2013-05-02 1:12 ` Dave Chinner 2013-05-02 6:55 ` Jeff Liu 0 siblings, 1 reply; 3+ messages in thread From: Dave Chinner @ 2013-05-02 1:12 UTC (permalink / raw) To: Jeff Liu; +Cc: Mark Tinguely, xfs@oss.sgi.com On Wed, May 01, 2013 at 10:25:13PM +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 confirmation. > > 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 | 39 ++++++++++++--------------------------- > fs/xfs/xfs_mount.h | 2 -- > fs/xfs/xfs_super.c | 5 +---- > 3 files changed, 13 insertions(+), 33 deletions(-) > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index 3806088..29e8de8 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)"); 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); > + 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)", "alignment check failed: sunit/swidth vs. agsize(%d)", sbp->sb_agblocks); > + 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); "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 bc90706..8145412 100644 > --- a/fs/xfs/xfs_mount.h > +++ b/fs/xfs/xfs_mount.h > @@ -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 */ > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index ea341ce..51da4d2 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -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; 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. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] xfs: Remove XFS_MOUNT_RETERR 2013-05-02 1:12 ` Dave Chinner @ 2013-05-02 6:55 ` Jeff Liu 0 siblings, 0 replies; 3+ messages in thread From: Jeff Liu @ 2013-05-02 6:55 UTC (permalink / raw) To: Dave Chinner; +Cc: Mark Tinguely, xfs@oss.sgi.com On 05/02/2013 09:12 AM, Dave Chinner wrote: > On Wed, May 01, 2013 at 10:25:13PM +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 confirmation. >> >> 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 | 39 ++++++++++++--------------------------- >> fs/xfs/xfs_mount.h | 2 -- >> fs/xfs/xfs_super.c | 5 +---- >> 3 files changed, 13 insertions(+), 33 deletions(-) >> >> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c >> index 3806088..29e8de8 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)"); > > 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); Ok. > >> + 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)", > > "alignment check failed: sunit/swidth vs. agsize(%d)", > sbp->sb_agblocks); > >> + 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); > > "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 bc90706..8145412 100644 >> --- a/fs/xfs/xfs_mount.h >> +++ b/fs/xfs/xfs_mount.h >> @@ -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 */ >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c >> index ea341ce..51da4d2 100644 >> --- a/fs/xfs/xfs_super.c >> +++ b/fs/xfs/xfs_super.c >> @@ -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; > > 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; > } Exactly, will take care of this in next round of post. Thanks, -Jeff _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-05-02 6:55 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-01 14:25 [PATCH 1/2] xfs: Remove XFS_MOUNT_RETERR Jeff Liu 2013-05-02 1:12 ` Dave Chinner 2013-05-02 6:55 ` Jeff Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox