xfs
[Top] [All Lists]

Re: [PATCH] log reasons for mount-time sunit/swidth rejection

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] log reasons for mount-time sunit/swidth rejection
From: Mark Goodwin <markgw@xxxxxxx>
Date: Thu, 11 Sep 2008 15:51:15 +1000
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <48C73592.6050806@sandeen.net>
Organization: SGI Engineering
References: <48C73592.6050806@sandeen.net>
Reply-to: markgw@xxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.16 (Windows/20080708)
Thanks Eric. Tim has been poking around this code recently
(investigating an interaction with a volume manager). Tim
can you review?  Thanks .. (ditto for the next patch).

Eric Sandeen wrote:
In xfs_mountfs ....

        /*
         * Check if sb_agblocks is aligned at stripe boundary
         * If sb_agblocks is NOT aligned turn off m_dalign since
         * allocator alignment is within an ag, therefore ag has
         * to be aligned at stripe boundary.
         */
        update_flags = 0LL;
        if (mp->m_dalign && !(mfsi_flags & XFS_MFSI_SECOND)) {
                /*
                 * If stripe unit and stripe width are not multiples
                 * of the fs blocksize turn off alignment.
                 */
                if ((BBTOB(mp->m_dalign) & mp->m_blockmask) ||
                    (BBTOB(mp->m_swidth) & mp->m_blockmask)) {
                        if (mp->m_flags & XFS_MOUNT_RETERR) {
                                cmn_err(CE_WARN,
                                        "XFS: alignment check 1 failed");
                                error = XFS_ERROR(EINVAL);
                                goto error1;
                        }

^^^^ here we fail with an oh-so-helpful warning

                        mp->m_dalign = mp->m_swidth = 0;
                } 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) {
                                        error = XFS_ERROR(EINVAL);
                                        goto error1;
                                }

^^^ here we fail with no message from mount whatsoever!

                                xfs_fs_cmn_err(CE_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;
                        } 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_fs_cmn_err(CE_WARN, mp,
"stripe alignment turned off: sunit(%d) less than bsize(%d)",
                                                mp->m_dalign,
                                                mp->m_blockmask +1);
                                        error = XFS_ERROR(EINVAL);
                                        goto error1;
                                }

^^^ here we fail with a misleading message (it's not turned off; it's a failure
and we return as such).

                                mp->m_swidth = 0;
                        }
                }

http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_mount.c.diff?r1=1.342;r2=1.343

Did that commit just misplace one of the error messages?

I'm thinking that we should certainly printk a message in all cases, and the "turned off" messages should only come if !XFS_MOUNT_RETERR; something like this?
(might want to clean up messages or cmn_err vs. xfs_fs_cmn_err or whatnot,
but you get the idea ....


-----------------------------------

XFS: log reasons for mount-time sunit/swidth rejection

When mount-time sunit/swidth are deemed incompatible with fs
geometry, leave a message in the logs rather than failing
silently.

Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
---


Index: linux-2.6.26.x86_64/fs/xfs/xfs_mount.c
===================================================================
--- linux-2.6.26.x86_64.orig/fs/xfs/xfs_mount.c
+++ linux-2.6.26.x86_64/fs/xfs/xfs_mount.c
@@ -745,11 +745,13 @@ xfs_update_alignment(xfs_mount_t *mp, in
*/
if ((BBTOB(mp->m_dalign) & mp->m_blockmask) ||
(BBTOB(mp->m_swidth) & mp->m_blockmask)) {
- if (mp->m_flags & XFS_MOUNT_RETERR) {
- cmn_err(CE_WARN,
- "XFS: alignment check 1 failed");
+ cmn_err(CE_WARN,
+ "XFS: sunit (%d) or swidth (%d) not "
+ "blocksize (%d) multiples",
+ mp->m_dalign, mp->m_swidth,
+ mp->m_blockmask +1);
+ if (mp->m_flags & XFS_MOUNT_RETERR)
return XFS_ERROR(EINVAL);
- }
mp->m_dalign = mp->m_swidth = 0;
} else {
/*
@@ -757,28 +759,26 @@ xfs_update_alignment(xfs_mount_t *mp, in
*/
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) {
- return XFS_ERROR(EINVAL);
- }
xfs_fs_cmn_err(CE_WARN, mp,
-"stripe alignment turned off: sunit(%d)/swidth(%d) incompatible with agsize(%d)",
- mp->m_dalign, mp->m_swidth,
- sbp->sb_agblocks);
-
+ "agsize (%d) not multiple of sunit (%d)",
+ sbp->sb_agblocks, mp->m_dalign);
+ if (mp->m_flags & XFS_MOUNT_RETERR)
+ return XFS_ERROR(EINVAL);
mp->m_dalign = 0;
mp->m_swidth = 0;
} 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_fs_cmn_err(CE_WARN, mp,
-"stripe alignment turned off: sunit(%d) less than bsize(%d)",
- mp->m_dalign,
- mp->m_blockmask +1);
+ xfs_fs_cmn_err(CE_WARN, mp,
+ "sunit (%d) less than bsize (%d)",
+ mp->m_dalign, mp->m_blockmask +1);
+ if (mp->m_flags & XFS_MOUNT_RETERR)
return XFS_ERROR(EINVAL);
- }
mp->m_swidth = 0;
}
+ if (mp->m_dalign == 0 && mp->m_swidth == 0)
+ xfs_fs_cmn_err(CE_WARN, mp,
+ "stripe aligment turned off.");
}
/*




--

 Mark Goodwin                                  markgw@xxxxxxx
 Engineering Manager for XFS and PCP    Phone: +61-3-99631937
 SGI Australian Software Group           Cell: +61-4-18969583
-------------------------------------------------------------


<Prev in Thread] Current Thread [Next in Thread>