Re: [RFC v6 PATCH 07/10] xfs: prevent fs freeze with outstanding relog items

Allison Collins
2020-04-09T00:05:29Z

On 4/6/20 5:36 AM, Brian Foster wrote:
> The automatic relog mechanism is currently incompatible with
> filesystem freeze in a generic sense. Freeze protection is currently
> implemented via locks that cannot be held on return to userspace,
> which means we can't hold a superblock write reference for the
> duration relogging is enabled on an item. It's too late to block
> when the freeze sequence calls into the filesystem because the
> transaction subsystem has already begun to be frozen. Not only can
> this block the relog transaction, but blocking any unrelated
> transaction essentially prevents a particular operation from
> progressing to the point where it can disable relogging on an item.
> Therefore marking the relog transaction as "nowrite" does not solve
> the problem.
>
> This is not a problem in practice because the two primary use cases
> already exclude freeze via other means. quotaoff holds ->s_umount
> read locked across the operation and scrub explicitly takes a
> superblock write reference, both of which block freeze of the
> transaction subsystem for the duration of relog enabled items.
>
> As a fallback for future use cases and the upcoming random buffer
> relogging test code, fail fs freeze attempts when the global relog
> reservation counter is elevated to prevent deadlock. This is a
> partial punt of the problem as compared to teaching freeze to wait
> on relogged items because the only current dependency is test code.
> In other words, this patch prevents deadlock if a user happens to
> issue a freeze in conjunction with random buffer relog injection.
>
> Signed-off-by: Brian Foster <bfoster@redhat.com>
Ok, long explanation, but makes sense :-)

Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> ---
> fs/xfs/xfs_super.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index abf06bf9c3f3..0efa9dc70d71 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -35,6 +35,7 @@
> #include "xfs_refcount_item.h"
> #include "xfs_bmap_item.h"
> #include "xfs_reflink.h"
> +#include "xfs_trans_priv.h"
>
> #include <linux/magic.h>
> #include <linux/fs_context.h>
> @@ -870,6 +871,9 @@ xfs_fs_freeze(
> {
> struct xfs_mount *mp = XFS_M(sb);
>
> + if (WARN_ON_ONCE(atomic64_read(&mp->m_ail->ail_relog_res)))
> + return -EAGAIN;
> +
> xfs_stop_block_reaping(mp);
> xfs_save_resvblks(mp);
> xfs_quiesce_attr(mp);
>