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 ... ?

Reply via email to