On Wed, Jun 22, 2022 at 7:38 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Fri, Jun 17, 2022 at 01:06:39PM +0300, Amir Goldstein wrote:
> > From: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> >
> > commit b97cca3ba9098522e5a1c3388764ead42640c1a5 upstream.
> >
> > In commit 02b9984d6408, we pushed a sync_filesystem() call from the VFS
> > into xfs_fs_remount. The only time that we ever need to push dirty file
> > data or metadata to disk for a remount is if we're remounting the
> > filesystem read only, so this really could be moved to xfs_remount_ro.
> >
> > Once we've moved the call site, actually check the return value from
> > sync_filesystem.
This part is not really relevant for this backport, do you want me to
emphasise that?
> >
> > Fixes: 02b9984d6408 ("fs: push sync_filesystem() down to the file system's remount_fs()")
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> > fs/xfs/xfs_super.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 6323974d6b3e..dd0439ae6732 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -1716,6 +1716,11 @@ xfs_remount_ro(
> > };
> > int error;
> >
> > + /* Flush all the dirty data to disk. */
> > + error = sync_filesystem(mp->m_super);
>
> Looking at 5.10.124's fsync.c and xfs_super.c:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/fs/sync.c?h=v5.10.124#n31
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/fs/xfs/xfs_super.c?h=v5.10.124#n755
>
> I think this kernel needs the patch(es) that make __sync_filesystem return
> the errors passed back by ->sync_fs, and I think also the patch that
> makes xfs_fs_sync_fs return errors encountered by xfs_log_force, right?
It wasn't my intention to fix syncfs() does not return errors in 5.10.
It has always been that way and IIRC, the relevant patches did not
apply cleanly.
THIS patch however, fixes something else, not only the return of the error
to its caller, so I thought it was worth backporting.
If you think otherwise, I'll drop it.
Thanks,
Amir.
