xfs
[Top] [All Lists]

Re: [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed vi

To: Jeff Liu <jeff.liu@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: Don't keep silent if sunit/swidth can not be changed via mount
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 30 Apr 2013 21:35:14 +1000
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <517F78D2.5020503@xxxxxxxxxx>
References: <517E5D12.5010809@xxxxxxxxxx> <20130430073659.GH23072@dastard> <517F78D2.5020503@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxxxxx

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