* [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount
@ 2013-04-29 11:44 Jeff Liu
2013-04-29 14:51 ` Mark Tinguely
2013-04-30 7:36 ` Dave Chinner
0 siblings, 2 replies; 6+ messages in thread
From: Jeff Liu @ 2013-04-29 11:44 UTC (permalink / raw)
To: xfs@oss.sgi.com
From: Jie Liu <jeff.liu@oracle.com>
As per the mount man page, sunit and swidth can be changed via
mount options. For XFS, on the face of it, those options seems
works if the specified alignments is properly, e.g.
# mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
# mount | grep sdb1
/dev/sdb1 on /mnt type xfs (rw,sunit=4096,swidth=8192)
However, neither sunit nor swidth is shown from the xfs_info output.
# xfs_info /mnt
meta-data=/dev/sdb1 isize=256 agcount=4, agsize=262144 blks
= sectsz=512 attr=2
data = bsize=4096 blocks=1048576, imaxpct=25
= sunit=0 swidth=0 blks
^^^^^^^^^^^^^^^^^^^^^^^^^^
naming =version 2 bsize=4096 ascii-ci=0
log =internal bsize=4096 blocks=2560, version=2
= sectsz=512 sunit=0 blks, lazy-count=1
realtime =none extsz=4096 blocks=0, rtextents=0
The reason is that the alignment can only be changed if the relevant
super block is already configured with alignments, otherwise, the
given value will be silently ignored, so it's better to tell user
that the alignment-changing can not take affect in one way or another.
With this fix, the attempt to mount a storage without strip alignment
setup on super block will failed if XFS_MOUNT_RETERR is enabled, or
just ignore the given alignment and drop a warning to indicate the
cause in syslog.
# mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
mount: wrong fs type, bad option, bad superblock on /dev/sdb1,
missing codepage or helper program, or other error
In some cases useful info is found in syslog - try
dmesg | tail or so
# dmesg|tail
.......
XFS (sdb1): can not change alignment: no data alignment on superblock
Signed-off-by: Jie Liu <jeff.liu@oracle.com>
---
fs/xfs/xfs_mount.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 3806088..bc7fdd4 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -924,6 +924,13 @@ xfs_update_alignment(xfs_mount_t *mp)
sbp->sb_width = mp->m_swidth;
mp->m_update_flags |= XFS_SB_WIDTH;
}
+ } else {
+ xfs_warn(mp, "can not change alignment: "
+ "no data alignment on superblock");
+ if (mp->m_flags & XFS_MOUNT_RETERR)
+ return XFS_ERROR(EINVAL);
+ mp->m_dalign = 0;
+ mp->m_swidth = 0;
}
} else if ((mp->m_flags & XFS_MOUNT_NOALIGN) != XFS_MOUNT_NOALIGN &&
xfs_sb_version_hasdalign(&mp->m_sb)) {
--
1.7.9.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount
2013-04-29 11:44 [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount Jeff Liu
@ 2013-04-29 14:51 ` Mark Tinguely
2013-04-30 2:11 ` Jeff Liu
2013-04-30 7:36 ` Dave Chinner
1 sibling, 1 reply; 6+ messages in thread
From: Mark Tinguely @ 2013-04-29 14:51 UTC (permalink / raw)
To: Jeff Liu; +Cc: xfs@oss.sgi.com
On 04/29/13 06:44, Jeff Liu wrote:
> From: Jie Liu<jeff.liu@oracle.com>
>
> As per the mount man page, sunit and swidth can be changed via
> mount options. For XFS, on the face of it, those options seems
> works if the specified alignments is properly, e.g.
> # mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
> # mount | grep sdb1
> /dev/sdb1 on /mnt type xfs (rw,sunit=4096,swidth=8192)
>
> However, neither sunit nor swidth is shown from the xfs_info output.
> # xfs_info /mnt
> meta-data=/dev/sdb1 isize=256 agcount=4, agsize=262144 blks
> = sectsz=512 attr=2
> data = bsize=4096 blocks=1048576, imaxpct=25
> = sunit=0 swidth=0 blks
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> naming =version 2 bsize=4096 ascii-ci=0
> log =internal bsize=4096 blocks=2560, version=2
> = sectsz=512 sunit=0 blks, lazy-count=1
> realtime =none extsz=4096 blocks=0, rtextents=0
>
> The reason is that the alignment can only be changed if the relevant
> super block is already configured with alignments, otherwise, the
> given value will be silently ignored, so it's better to tell user
> that the alignment-changing can not take affect in one way or another.
>
> With this fix, the attempt to mount a storage without strip alignment
> setup on super block will failed if XFS_MOUNT_RETERR is enabled, or
> just ignore the given alignment and drop a warning to indicate the
> cause in syslog.
>
> # mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
> mount: wrong fs type, bad option, bad superblock on /dev/sdb1,
> missing codepage or helper program, or other error
> In some cases useful info is found in syslog - try
> dmesg | tail or so
>
> # dmesg|tail
> .......
> XFS (sdb1): can not change alignment: no data alignment on superblock
>
> Signed-off-by: Jie Liu<jeff.liu@oracle.com>
>
> ---
> fs/xfs/xfs_mount.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 3806088..bc7fdd4 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -924,6 +924,13 @@ xfs_update_alignment(xfs_mount_t *mp)
> sbp->sb_width = mp->m_swidth;
> mp->m_update_flags |= XFS_SB_WIDTH;
> }
> + } else {
> + xfs_warn(mp, "can not change alignment: "
> + "no data alignment on superblock");
suggest that you keep the string together so it can be searched.
"superblock does not support data alignment"
> + if (mp->m_flags& XFS_MOUNT_RETERR)
> + return XFS_ERROR(EINVAL);
> + mp->m_dalign = 0;
> + mp->m_swidth = 0;
> }
> } else if ((mp->m_flags& XFS_MOUNT_NOALIGN) != XFS_MOUNT_NOALIGN&&
> xfs_sb_version_hasdalign(&mp->m_sb)) {
Thanks,
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount
2013-04-29 14:51 ` Mark Tinguely
@ 2013-04-30 2:11 ` Jeff Liu
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Liu @ 2013-04-30 2:11 UTC (permalink / raw)
To: Mark Tinguely; +Cc: xfs@oss.sgi.com
On 04/29/2013 10:51 PM, Mark Tinguely wrote:
> On 04/29/13 06:44, Jeff Liu wrote:
>> From: Jie Liu<jeff.liu@oracle.com>
>>
>> As per the mount man page, sunit and swidth can be changed via
>> mount options. For XFS, on the face of it, those options seems
>> works if the specified alignments is properly, e.g.
>> # mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
>> # mount | grep sdb1
>> /dev/sdb1 on /mnt type xfs (rw,sunit=4096,swidth=8192)
>>
>> However, neither sunit nor swidth is shown from the xfs_info output.
>> # xfs_info /mnt
>> meta-data=/dev/sdb1 isize=256 agcount=4, agsize=262144 blks
>> = sectsz=512 attr=2
>> data = bsize=4096 blocks=1048576, imaxpct=25
>> = sunit=0 swidth=0 blks
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^
>> naming =version 2 bsize=4096 ascii-ci=0
>> log =internal bsize=4096 blocks=2560, version=2
>> = sectsz=512 sunit=0 blks, lazy-count=1
>> realtime =none extsz=4096 blocks=0, rtextents=0
>>
>> The reason is that the alignment can only be changed if the relevant
>> super block is already configured with alignments, otherwise, the
>> given value will be silently ignored, so it's better to tell user
>> that the alignment-changing can not take affect in one way or another.
>>
>> With this fix, the attempt to mount a storage without strip alignment
>> setup on super block will failed if XFS_MOUNT_RETERR is enabled, or
>> just ignore the given alignment and drop a warning to indicate the
>> cause in syslog.
>>
>> # mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
>> mount: wrong fs type, bad option, bad superblock on /dev/sdb1,
>> missing codepage or helper program, or other error
>> In some cases useful info is found in syslog - try
>> dmesg | tail or so
>>
>> # dmesg|tail
>> .......
>> XFS (sdb1): can not change alignment: no data alignment on superblock
>>
>> Signed-off-by: Jie Liu<jeff.liu@oracle.com>
>>
>> ---
>> fs/xfs/xfs_mount.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
>> index 3806088..bc7fdd4 100644
>> --- a/fs/xfs/xfs_mount.c
>> +++ b/fs/xfs/xfs_mount.c
>> @@ -924,6 +924,13 @@ xfs_update_alignment(xfs_mount_t *mp)
>> sbp->sb_width = mp->m_swidth;
>> mp->m_update_flags |= XFS_SB_WIDTH;
>> }
>> + } else {
>> + xfs_warn(mp, "can not change alignment: "
>> + "no data alignment on superblock");
> suggest that you keep the string together so it can be searched.
> "superblock does not support data alignment"
Do you means that the string would shown as: "superblock does not
support data alignment"?
Or "cannot change alignment: superblock does not support data alignment"?
I prefer to the later format because it can distinguish this special
case from the previous alignment validation procedure, i.e,
"alignment check failed: xxx".
Thanks,
-Jeff
>
>> + if (mp->m_flags& XFS_MOUNT_RETERR)
>> + return XFS_ERROR(EINVAL);
>> + mp->m_dalign = 0;
>> + mp->m_swidth = 0;
>> }
>> } else if ((mp->m_flags& XFS_MOUNT_NOALIGN) != XFS_MOUNT_NOALIGN&&
>> xfs_sb_version_hasdalign(&mp->m_sb)) {
>
>
> Thanks,
>
> --Mark.
>
> _______________________________________________
> 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] 6+ messages in thread
* Re: [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount
2013-04-29 11:44 [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount Jeff Liu
2013-04-29 14:51 ` Mark Tinguely
@ 2013-04-30 7:36 ` Dave Chinner
2013-04-30 7:54 ` Jeff Liu
1 sibling, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2013-04-30 7:36 UTC (permalink / raw)
To: Jeff Liu; +Cc: xfs@oss.sgi.com
On Mon, Apr 29, 2013 at 07:44:18PM +0800, Jeff Liu wrote:
> From: Jie Liu <jeff.liu@oracle.com>
>
> As per the mount man page, sunit and swidth can be changed via
> mount options. For XFS, on the face of it, those options seems
> works if the specified alignments is properly, e.g.
> # mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
> # mount | grep sdb1
> /dev/sdb1 on /mnt type xfs (rw,sunit=4096,swidth=8192)
>
> However, neither sunit nor swidth is shown from the xfs_info output.
> # xfs_info /mnt
> meta-data=/dev/sdb1 isize=256 agcount=4, agsize=262144 blks
> = sectsz=512 attr=2
> data = bsize=4096 blocks=1048576, imaxpct=25
> = sunit=0 swidth=0 blks
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> naming =version 2 bsize=4096 ascii-ci=0
> log =internal bsize=4096 blocks=2560, version=2
> = sectsz=512 sunit=0 blks, lazy-count=1
> realtime =none extsz=4096 blocks=0, rtextents=0
>
> The reason is that the alignment can only be changed if the relevant
> super block is already configured with alignments, otherwise, the
> given value will be silently ignored, so it's better to tell user
> that the alignment-changing can not take affect in one way or another.
>
> With this fix, the attempt to mount a storage without strip alignment
> setup on super block will failed if XFS_MOUNT_RETERR is enabled, or
> just ignore the given alignment and drop a warning to indicate the
> cause in syslog.
>
> # mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
> mount: wrong fs type, bad option, bad superblock on /dev/sdb1,
> missing codepage or helper program, or other error
> In some cases useful info is found in syslog - try
> dmesg | tail or so
>
> # dmesg|tail
> .......
> XFS (sdb1): can not change alignment: no data alignment on superblock
>
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>
> ---
> fs/xfs/xfs_mount.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 3806088..bc7fdd4 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -924,6 +924,13 @@ xfs_update_alignment(xfs_mount_t *mp)
> sbp->sb_width = mp->m_swidth;
> mp->m_update_flags |= XFS_SB_WIDTH;
> }
> + } else {
> + xfs_warn(mp, "can not change alignment: "
> + "no data alignment on superblock");
> + if (mp->m_flags & XFS_MOUNT_RETERR)
> + return XFS_ERROR(EINVAL);
> + mp->m_dalign = 0;
> + mp->m_swidth = 0;
Can someone tell me why the XFS_MOUNT_RETERR flag exists?
It looks like dead code to me as the only time mp->m_dalign is set
prior to calling xfs_update_alignment() is the same code that sets
XFS_MOUNT_RETERR in xfs_parseargs().
IOWs, any time we enter this "if (mp->m_dalign)" branch in
xfs_update_alignment(), XFS_MOUNT_RETERR is going to be set and so
we should always be emitting a warning and returning an error.
If this is correct, Jeff, can you remove the XFS_MOUNT_RETERR flag
and get rid of all the dead code in xfs_update_alignment() at the
same time, please?
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] 6+ messages in thread
* Re: [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount
2013-04-30 7:36 ` Dave Chinner
@ 2013-04-30 7:54 ` Jeff Liu
2013-04-30 11:35 ` Dave Chinner
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Liu @ 2013-04-30 7:54 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs@oss.sgi.com
On 04/30/2013 03:36 PM, Dave Chinner wrote:
> On Mon, Apr 29, 2013 at 07:44:18PM +0800, Jeff Liu wrote:
>> From: Jie Liu <jeff.liu@oracle.com>
>>
>> As per the mount man page, sunit and swidth can be changed via
>> mount options. For XFS, on the face of it, those options seems
>> works if the specified alignments is properly, e.g.
>> # mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
>> # mount | grep sdb1
>> /dev/sdb1 on /mnt type xfs (rw,sunit=4096,swidth=8192)
>>
>> However, neither sunit nor swidth is shown from the xfs_info output.
>> # xfs_info /mnt
>> meta-data=/dev/sdb1 isize=256 agcount=4, agsize=262144 blks
>> = sectsz=512 attr=2
>> data = bsize=4096 blocks=1048576, imaxpct=25
>> = sunit=0 swidth=0 blks
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^
>> naming =version 2 bsize=4096 ascii-ci=0
>> log =internal bsize=4096 blocks=2560, version=2
>> = sectsz=512 sunit=0 blks, lazy-count=1
>> realtime =none extsz=4096 blocks=0, rtextents=0
>>
>> The reason is that the alignment can only be changed if the relevant
>> super block is already configured with alignments, otherwise, the
>> given value will be silently ignored, so it's better to tell user
>> that the alignment-changing can not take affect in one way or another.
>>
>> With this fix, the attempt to mount a storage without strip alignment
>> setup on super block will failed if XFS_MOUNT_RETERR is enabled, or
>> just ignore the given alignment and drop a warning to indicate the
>> cause in syslog.
>>
>> # mount -o sunit=4096,swidth=8192 /dev/sdb1 /mnt
>> mount: wrong fs type, bad option, bad superblock on /dev/sdb1,
>> missing codepage or helper program, or other error
>> In some cases useful info is found in syslog - try
>> dmesg | tail or so
>>
>> # dmesg|tail
>> .......
>> XFS (sdb1): can not change alignment: no data alignment on superblock
>>
>> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>>
>> ---
>> fs/xfs/xfs_mount.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
>> index 3806088..bc7fdd4 100644
>> --- a/fs/xfs/xfs_mount.c
>> +++ b/fs/xfs/xfs_mount.c
>> @@ -924,6 +924,13 @@ xfs_update_alignment(xfs_mount_t *mp)
>> sbp->sb_width = mp->m_swidth;
>> mp->m_update_flags |= XFS_SB_WIDTH;
>> }
>> + } else {
>> + xfs_warn(mp, "can not change alignment: "
>> + "no data alignment on superblock");
>> + if (mp->m_flags & XFS_MOUNT_RETERR)
>> + return XFS_ERROR(EINVAL);
>> + mp->m_dalign = 0;
>> + mp->m_swidth = 0;
>
> Can someone tell me why the XFS_MOUNT_RETERR flag exists?
This is really a very opportune response because I also worked out
another tiny patch for removing XFS_MOUNT_RETERR a few minutes ago,
just hesitating if I missed anything or not.
>
> It looks like dead code to me as the only time mp->m_dalign is set
> prior to calling xfs_update_alignment() is the same code that sets
> XFS_MOUNT_RETERR in xfs_parseargs().
>
> IOWs, any time we enter this "if (mp->m_dalign)" branch in
> xfs_update_alignment(), XFS_MOUNT_RETERR is going to be set and so
> we should always be emitting a warning and returning an error.
Yes, I realized that as I can not trigger a warning only, it always
returning an error to me. :(
>
> If this is correct, Jeff, can you remove the XFS_MOUNT_RETERR flag
> and get rid of all the dead code in xfs_update_alignment() at the
> same time, please?
Sure, I'll post this patch tonight together with another initial patch
for fixing transaction space over-reservation we have discussed two
weeks ago, xfstests is running now.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount
2013-04-30 7:54 ` Jeff Liu
@ 2013-04-30 11:35 ` Dave Chinner
0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2013-04-30 11:35 UTC (permalink / raw)
To: Jeff Liu; +Cc: xfs@oss.sgi.com
On Tue, Apr 30, 2013 at 03:54:58PM +0800, Jeff Liu wrote:
> On 04/30/2013 03:36 PM, Dave Chinner wrote:
> > On Mon, Apr 29, 2013 at 07:44:18PM +0800, Jeff Liu wrote:
> >> ---
> >> fs/xfs/xfs_mount.c | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> >> index 3806088..bc7fdd4 100644
> >> --- a/fs/xfs/xfs_mount.c
> >> +++ b/fs/xfs/xfs_mount.c
> >> @@ -924,6 +924,13 @@ xfs_update_alignment(xfs_mount_t *mp)
> >> sbp->sb_width = mp->m_swidth;
> >> mp->m_update_flags |= XFS_SB_WIDTH;
> >> }
> >> + } else {
> >> + xfs_warn(mp, "can not change alignment: "
> >> + "no data alignment on superblock");
> >> + if (mp->m_flags & XFS_MOUNT_RETERR)
> >> + return XFS_ERROR(EINVAL);
> >> + mp->m_dalign = 0;
> >> + mp->m_swidth = 0;
> >
> > Can someone tell me why the XFS_MOUNT_RETERR flag exists?
> This is really a very opportune response because I also worked out
> another tiny patch for removing XFS_MOUNT_RETERR a few minutes ago,
> just hesitating if I missed anything or not.
Excellent - you're one step ahead of me :)
> > It looks like dead code to me as the only time mp->m_dalign is set
> > prior to calling xfs_update_alignment() is the same code that sets
> > XFS_MOUNT_RETERR in xfs_parseargs().
> >
> > IOWs, any time we enter this "if (mp->m_dalign)" branch in
> > xfs_update_alignment(), XFS_MOUNT_RETERR is going to be set and so
> > we should always be emitting a warning and returning an error.
> Yes, I realized that as I can not trigger a warning only, it always
> returning an error to me. :(
> >
> > If this is correct, Jeff, can you remove the XFS_MOUNT_RETERR flag
> > and get rid of all the dead code in xfs_update_alignment() at the
> > same time, please?
> Sure, I'll post this patch tonight together with another initial patch
> for fixing transaction space over-reservation we have discussed two
> weeks ago, xfstests is running now.
I'll have a look when it appears on the list ;)
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] 6+ messages in thread
end of thread, other threads:[~2013-04-30 11:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-29 11:44 [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount Jeff Liu
2013-04-29 14:51 ` Mark Tinguely
2013-04-30 2:11 ` Jeff Liu
2013-04-30 7:36 ` Dave Chinner
2013-04-30 7:54 ` Jeff Liu
2013-04-30 11:35 ` Dave Chinner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox