Re: [PATCH v6 07/12] xfs: add xfs_remount_ro() helper

Darrick J. Wong
2019-10-16T18:30:23Z

On Wed, Oct 16, 2019 at 08:41:22AM +0800, Ian Kent wrote:
> Factor the remount read only code into a helper to simplify the
> subsequent change from the super block method .remount_fs to the
> mount-api fs_context_operations method .reconfigure.
>
> Signed-off-by: Ian Kent <raven@themaw.net>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> ---
> fs/xfs/xfs_super.c | 73 +++++++++++++++++++++++++++++++---------------------
> 1 file changed, 43 insertions(+), 30 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index cdc19c2af50f..1f706cbf9624 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1275,6 +1275,47 @@ xfs_remount_rw(
> return 0;
> }
>
> +static int
> +xfs_remount_ro(
> + struct xfs_mount *mp)
> +{
> + int error;

Please indent the variable consistently with the parameter list.
Otherwise looks ok.

--D

> +
> + /*
> + * Cancel background eofb scanning so it cannot race with the
> + * final log force+buftarg wait and deadlock the remount.
> + */
> + xfs_stop_block_reaping(mp);
> +
> + /* Get rid of any leftover CoW reservations... */
> + error = xfs_icache_free_cowblocks(mp, NULL);
> + if (error) {
> + xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> + return error;
> + }
> +
> + /* Free the per-AG metadata reservation pool. */
> + error = xfs_fs_unreserve_ag_blocks(mp);
> + if (error) {
> + xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> + return error;
> + }
> +
> + /*
> + * Before we sync the metadata, we need to free up the reserve
> + * block pool so that the used block count in the superblock on
> + * disk is correct at the end of the remount. Stash the current
> + * reserve pool size so that if we get remounted rw, we can
> + * return it to the same size.
> + */
> + xfs_save_resvblks(mp);
> +
> + xfs_quiesce_attr(mp);
> + mp->m_flags |= XFS_MOUNT_RDONLY;
> +
> + return 0;
> +}
> +
> STATIC int
> xfs_fs_remount(
> struct super_block *sb,
> @@ -1345,37 +1386,9 @@ xfs_fs_remount(
>
> /* rw -> ro */
> if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (*flags & SB_RDONLY)) {
> - /*
> - * Cancel background eofb scanning so it cannot race with the
> - * final log force+buftarg wait and deadlock the remount.
> - */
> - xfs_stop_block_reaping(mp);
> -
> - /* Get rid of any leftover CoW reservations... */
> - error = xfs_icache_free_cowblocks(mp, NULL);
> - if (error) {
> - xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> - return error;
> - }
> -
> - /* Free the per-AG metadata reservation pool. */
> - error = xfs_fs_unreserve_ag_blocks(mp);
> - if (error) {
> - xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
> + error = xfs_remount_ro(mp);
> + if (error)
> return error;
> - }
> -
> - /*
> - * Before we sync the metadata, we need to free up the reserve
> - * block pool so that the used block count in the superblock on
> - * disk is correct at the end of the remount. Stash the current
> - * reserve pool size so that if we get remounted rw, we can
> - * return it to the same size.
> - */
> - xfs_save_resvblks(mp);
> -
> - xfs_quiesce_attr(mp);
> - mp->m_flags |= XFS_MOUNT_RDONLY;
> }
>
> return 0;
>