diff mbox series

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

Message ID 20200406123632.20873-8-bfoster@redhat.com (mailing list archive)
State Superseded
Headers show
Series xfs: automatic relogging experiment | expand

Commit Message

Brian Foster April 6, 2020, 12:36 p.m. UTC
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>
---
 fs/xfs/xfs_super.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Allison Henderson April 9, 2020, 12:05 a.m. UTC | #1
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);
>
diff mbox series

Patch

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);