| Message ID | 160212937238.248573.3832120826354421788.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: a couple of realtime growfs fixes | expand |
On Thursday 8 October 2020 9:26:12 AM IST Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > 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@oracle.com> > --- > fs/xfs/xfs_rtalloc.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c > index 1c3969807fb9..5b2e68d9face 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, > @@ -1108,6 +1108,11 @@ xfs_growfs_rt( > */ > kmem_free(nmp); > > + /* Update secondary superblocks now the physical grow has completed */ > + error = xfs_update_secondary_sbs(mp); > + if (error) > + return error; > + If any of the operations in the previous "for" loop causes "error" to be set and the loop to be exited, the call to xfs_update_secondary_sbs() would overwrite this error value. In the worst case it might set error to 0 and hence return a success status to the caller when the growfs operation had actually failed.
On Thu, Oct 08, 2020 at 01:01:59PM +0530, Chandan Babu R wrote: > On Thursday 8 October 2020 9:26:12 AM IST Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > 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@oracle.com> > > --- > > fs/xfs/xfs_rtalloc.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c > > index 1c3969807fb9..5b2e68d9face 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, > > @@ -1108,6 +1108,11 @@ xfs_growfs_rt( > > */ > > kmem_free(nmp); > > > > + /* Update secondary superblocks now the physical grow has completed */ > > + error = xfs_update_secondary_sbs(mp); > > + if (error) > > + return error; > > + > > If any of the operations in the previous "for" loop causes "error" to be set > and the loop to be exited, the call to xfs_update_secondary_sbs() would > overwrite this error value. In the worst case it might set error to 0 and > hence return a success status to the caller when the growfs operation > had actually failed. Oops, good catch! --D > -- > chandan > > >
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c index 1c3969807fb9..5b2e68d9face 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, @@ -1108,6 +1108,11 @@ xfs_growfs_rt( */ kmem_free(nmp); + /* Update secondary superblocks now the physical grow has completed */ + error = xfs_update_secondary_sbs(mp); + if (error) + return error; + /* * If we had to allocate a new rsum_cache, we either need to free the * old one (if we succeeded) or free the new one and restore the old one