diff mbox series

[09/12] xfs: place intent recovery under NOFS allocation context

Message ID 20240115230113.4080105-10-david@fromorbit.com (mailing list archive)
State New
Headers show
Series xfs: remove remaining kmem interfaces and GFP_NOFS usage | expand

Commit Message

Dave Chinner Jan. 15, 2024, 10:59 p.m. UTC
From: Dave Chinner <dchinner@redhat.com>

When recovery starts processing intents, all of the initial intent
allocations are done outside of transaction contexts. That means
they need to specifically use GFP_NOFS as we do not want memory
reclaim to attempt to run direct reclaim of filesystem objects while
we have lots of objects added into deferred operations.

Rather than use GFP_NOFS for these specific allocations, just place
the entire intent recovery process under NOFS context and we can
then just use GFP_KERNEL for these allocations.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_attr_item.c     |  2 +-
 fs/xfs/xfs_bmap_item.c     |  3 ++-
 fs/xfs/xfs_log_recover.c   | 18 ++++++++++++++----
 fs/xfs/xfs_refcount_item.c |  2 +-
 fs/xfs/xfs_rmap_item.c     |  2 +-
 5 files changed, 19 insertions(+), 8 deletions(-)

Comments

Darrick J. Wong Jan. 18, 2024, 11:39 p.m. UTC | #1
On Tue, Jan 16, 2024 at 09:59:47AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> When recovery starts processing intents, all of the initial intent
> allocations are done outside of transaction contexts. That means
> they need to specifically use GFP_NOFS as we do not want memory
> reclaim to attempt to run direct reclaim of filesystem objects while
> we have lots of objects added into deferred operations.
> 
> Rather than use GFP_NOFS for these specific allocations, just place
> the entire intent recovery process under NOFS context and we can
> then just use GFP_KERNEL for these allocations.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Hooray!  This finally goes away...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_attr_item.c     |  2 +-
>  fs/xfs/xfs_bmap_item.c     |  3 ++-
>  fs/xfs/xfs_log_recover.c   | 18 ++++++++++++++----
>  fs/xfs/xfs_refcount_item.c |  2 +-
>  fs/xfs/xfs_rmap_item.c     |  2 +-
>  5 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index 0bf25a2ba3b6..e14e229fc712 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -513,7 +513,7 @@ xfs_attri_recover_work(
>  		return ERR_PTR(error);
>  
>  	attr = kzalloc(sizeof(struct xfs_attr_intent) +
> -			sizeof(struct xfs_da_args), GFP_NOFS | __GFP_NOFAIL);
> +			sizeof(struct xfs_da_args), GFP_KERNEL | __GFP_NOFAIL);
>  	args = (struct xfs_da_args *)(attr + 1);
>  
>  	attr->xattri_da_args = args;
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 029a6a8d0efd..e3c58090e976 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -445,7 +445,8 @@ xfs_bui_recover_work(
>  	if (error)
>  		return ERR_PTR(error);
>  
> -	bi = kmem_cache_zalloc(xfs_bmap_intent_cache, GFP_NOFS | __GFP_NOFAIL);
> +	bi = kmem_cache_zalloc(xfs_bmap_intent_cache,
> +			GFP_KERNEL | __GFP_NOFAIL);
>  	bi->bi_whichfork = (map->me_flags & XFS_BMAP_EXTENT_ATTR_FORK) ?
>  			XFS_ATTR_FORK : XFS_DATA_FORK;
>  	bi->bi_type = map->me_flags & XFS_BMAP_EXTENT_TYPE_MASK;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index e9ed43a833af..8c1d260bb9e1 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -3443,12 +3443,19 @@ xlog_recover(
>   * part of recovery so that the root and real-time bitmap inodes can be read in
>   * from disk in between the two stages.  This is necessary so that we can free
>   * space in the real-time portion of the file system.
> + *
> + * We run this whole process under GFP_NOFS allocation context. We do a
> + * combination of non-transactional and transactional work, yet we really don't
> + * want to recurse into the filesystem from direct reclaim during any of this
> + * processing. This allows all the recovery code run here not to care about the
> + * memory allocation context it is running in.
>   */
>  int
>  xlog_recover_finish(
>  	struct xlog	*log)
>  {
> -	int	error;
> +	unsigned int	nofs_flags = memalloc_nofs_save();
> +	int		error;
>  
>  	error = xlog_recover_process_intents(log);
>  	if (error) {
> @@ -3462,7 +3469,7 @@ xlog_recover_finish(
>  		xlog_recover_cancel_intents(log);
>  		xfs_alert(log->l_mp, "Failed to recover intents");
>  		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
> -		return error;
> +		goto out_error;
>  	}
>  
>  	/*
> @@ -3483,7 +3490,7 @@ xlog_recover_finish(
>  		if (error < 0) {
>  			xfs_alert(log->l_mp,
>  	"Failed to clear log incompat features on recovery");
> -			return error;
> +			goto out_error;
>  		}
>  	}
>  
> @@ -3508,9 +3515,12 @@ xlog_recover_finish(
>  		 * and AIL.
>  		 */
>  		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
> +		goto out_error;
>  	}
>  
> -	return 0;
> +out_error:
> +	memalloc_nofs_restore(nofs_flags);
> +	return error;
>  }
>  
>  void
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index d850b9685f7f..14919b33e4fe 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -425,7 +425,7 @@ xfs_cui_recover_work(
>  	struct xfs_refcount_intent	*ri;
>  
>  	ri = kmem_cache_alloc(xfs_refcount_intent_cache,
> -			GFP_NOFS | __GFP_NOFAIL);
> +			GFP_KERNEL | __GFP_NOFAIL);
>  	ri->ri_type = pmap->pe_flags & XFS_REFCOUNT_EXTENT_TYPE_MASK;
>  	ri->ri_startblock = pmap->pe_startblock;
>  	ri->ri_blockcount = pmap->pe_len;
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index a40b92ac81e8..e473124e29cc 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -455,7 +455,7 @@ xfs_rui_recover_work(
>  {
>  	struct xfs_rmap_intent		*ri;
>  
> -	ri = kmem_cache_alloc(xfs_rmap_intent_cache, GFP_NOFS | __GFP_NOFAIL);
> +	ri = kmem_cache_alloc(xfs_rmap_intent_cache, GFP_KERNEL | __GFP_NOFAIL);
>  
>  	switch (map->me_flags & XFS_RMAP_EXTENT_TYPE_MASK) {
>  	case XFS_RMAP_EXTENT_MAP:
> -- 
> 2.43.0
> 
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index 0bf25a2ba3b6..e14e229fc712 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -513,7 +513,7 @@  xfs_attri_recover_work(
 		return ERR_PTR(error);
 
 	attr = kzalloc(sizeof(struct xfs_attr_intent) +
-			sizeof(struct xfs_da_args), GFP_NOFS | __GFP_NOFAIL);
+			sizeof(struct xfs_da_args), GFP_KERNEL | __GFP_NOFAIL);
 	args = (struct xfs_da_args *)(attr + 1);
 
 	attr->xattri_da_args = args;
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 029a6a8d0efd..e3c58090e976 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -445,7 +445,8 @@  xfs_bui_recover_work(
 	if (error)
 		return ERR_PTR(error);
 
-	bi = kmem_cache_zalloc(xfs_bmap_intent_cache, GFP_NOFS | __GFP_NOFAIL);
+	bi = kmem_cache_zalloc(xfs_bmap_intent_cache,
+			GFP_KERNEL | __GFP_NOFAIL);
 	bi->bi_whichfork = (map->me_flags & XFS_BMAP_EXTENT_ATTR_FORK) ?
 			XFS_ATTR_FORK : XFS_DATA_FORK;
 	bi->bi_type = map->me_flags & XFS_BMAP_EXTENT_TYPE_MASK;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index e9ed43a833af..8c1d260bb9e1 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3443,12 +3443,19 @@  xlog_recover(
  * part of recovery so that the root and real-time bitmap inodes can be read in
  * from disk in between the two stages.  This is necessary so that we can free
  * space in the real-time portion of the file system.
+ *
+ * We run this whole process under GFP_NOFS allocation context. We do a
+ * combination of non-transactional and transactional work, yet we really don't
+ * want to recurse into the filesystem from direct reclaim during any of this
+ * processing. This allows all the recovery code run here not to care about the
+ * memory allocation context it is running in.
  */
 int
 xlog_recover_finish(
 	struct xlog	*log)
 {
-	int	error;
+	unsigned int	nofs_flags = memalloc_nofs_save();
+	int		error;
 
 	error = xlog_recover_process_intents(log);
 	if (error) {
@@ -3462,7 +3469,7 @@  xlog_recover_finish(
 		xlog_recover_cancel_intents(log);
 		xfs_alert(log->l_mp, "Failed to recover intents");
 		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
-		return error;
+		goto out_error;
 	}
 
 	/*
@@ -3483,7 +3490,7 @@  xlog_recover_finish(
 		if (error < 0) {
 			xfs_alert(log->l_mp,
 	"Failed to clear log incompat features on recovery");
-			return error;
+			goto out_error;
 		}
 	}
 
@@ -3508,9 +3515,12 @@  xlog_recover_finish(
 		 * and AIL.
 		 */
 		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
+		goto out_error;
 	}
 
-	return 0;
+out_error:
+	memalloc_nofs_restore(nofs_flags);
+	return error;
 }
 
 void
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index d850b9685f7f..14919b33e4fe 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -425,7 +425,7 @@  xfs_cui_recover_work(
 	struct xfs_refcount_intent	*ri;
 
 	ri = kmem_cache_alloc(xfs_refcount_intent_cache,
-			GFP_NOFS | __GFP_NOFAIL);
+			GFP_KERNEL | __GFP_NOFAIL);
 	ri->ri_type = pmap->pe_flags & XFS_REFCOUNT_EXTENT_TYPE_MASK;
 	ri->ri_startblock = pmap->pe_startblock;
 	ri->ri_blockcount = pmap->pe_len;
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index a40b92ac81e8..e473124e29cc 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -455,7 +455,7 @@  xfs_rui_recover_work(
 {
 	struct xfs_rmap_intent		*ri;
 
-	ri = kmem_cache_alloc(xfs_rmap_intent_cache, GFP_NOFS | __GFP_NOFAIL);
+	ri = kmem_cache_alloc(xfs_rmap_intent_cache, GFP_KERNEL | __GFP_NOFAIL);
 
 	switch (map->me_flags & XFS_RMAP_EXTENT_TYPE_MASK) {
 	case XFS_RMAP_EXTENT_MAP: