Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of memalloc_noio*

From: Matthew Wilcox
Date: Mon Feb 06 2017 - 10:39:31 EST


On Mon, Feb 06, 2017 at 03:07:16PM +0100, Michal Hocko wrote:
> +++ b/fs/xfs/xfs_buf.c
> @@ -442,17 +442,17 @@ _xfs_buf_map_pages(
> bp->b_addr = NULL;
> } else {
> int retried = 0;
> - unsigned noio_flag;
> + unsigned nofs_flag;
>
> /*
> * vm_map_ram() will allocate auxillary structures (e.g.
> * pagetables) with GFP_KERNEL, yet we are likely to be under
> * GFP_NOFS context here. Hence we need to tell memory reclaim
> - * that we are in such a context via PF_MEMALLOC_NOIO to prevent
> + * that we are in such a context via PF_MEMALLOC_NOFS to prevent
> * memory reclaim re-entering the filesystem here and
> * potentially deadlocking.
> */

This comment feels out of date ... how about:

/*
* vm_map_ram will allocate auxiliary structures (eg page
* tables) with GFP_KERNEL. If that tries to reclaim memory
* by calling back into this filesystem, we may deadlock.
* Prevent that by setting the NOFS flag.
*/

> - noio_flag = memalloc_noio_save();
> + nofs_flag = memalloc_nofs_save();
> do {
> bp->b_addr = vm_map_ram(bp->b_pages, bp->b_page_count,
> -1, PAGE_KERNEL);

Also, I think it shows that this is the wrong place in XFS to be calling
memalloc_nofs_save(). I'm not arguing against including this patch;
it's a step towards where we want to be. I also don't know XFS well
enough to know where to set that flag ;-) Presumably when we start a
transaction ... ?