On Friday 9 October 2020 8:51:26 PM IST Darrick J. Wong wrote:
> On Fri, Oct 09, 2020 at 03:21:38PM +0530, Chandan Babu R wrote:
> > On Friday 9 October 2020 3:49:05 AM IST Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > >
> > > When we call growfs on the data device, we update the secondary
> > > superblocks to reflect the updated filesystem geometry. We need to do
> > > this for growfs on the realtime volume too, because a future xfs_repair
> > > run could try to fix the filesystem using a backup superblock.
> > >
> > > This was observed by the online superblock scrubbers while running
> > > xfs/233. One can also trigger this by growing an rt volume, cycling the
> > > mount, and creating new rt files.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > ---
> > > v2.2: don't update on error, don't fail to free memory on error
> > > ---
> > > fs/xfs/xfs_rtalloc.c | 8 +++++++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > > index 1c3969807fb9..f9119ba3e9d0 100644
> > > --- a/fs/xfs/xfs_rtalloc.c
> > > +++ b/fs/xfs/xfs_rtalloc.c
> > > @@ -18,7 +18,7 @@
> > > #include "xfs_trans_space.h"
> > > #include "xfs_icache.h"
> > > #include "xfs_rtalloc.h"
> > > -
> > > +#include "xfs_sb.h"
> > >
> > > /*
> > > * Read and return the summary information for a given extent size,
> > > @@ -1102,7 +1102,13 @@ xfs_growfs_rt(
> > > if (error)
> > > break;
> > > }
> > > + if (error)
> > > + goto out_free;
> > >
> > > + /* Update secondary superblocks now the physical grow has completed */
> > > + error = xfs_update_secondary_sbs(mp);
> > > +
> > > +out_free:
> > > /*
> > > * Free the fake mp structure.
> > > */
> > >
> >
> > How about ...
> >
> > if (!error) {
> > /* Update secondary superblocks now the physical grow has completed */
> > error = xfs_update_secondary_sbs(mp);
> > }
> >
> > /*
> > * Free the fake mp structure.
> > */
> > ...
> > ...
> >
> > With the above construct we can get rid of the goto label.
>
> I'd rather not start doing that, because (a) we generally don't do that
> in xfs and (b) in a cycle or two I'm going to add more in-memory state
> changes between the secondary super update and freeing the fake mp, and
> I'd prefer to start all that by having the error case jump to out_free.
>
Ok.
Reviewed-by: Chandan Babu R <chandanrlinux@xxxxxxxxx>
--
chandan
