Re: [PATCH 06/10] fs: iomap use memalloc_nofs_* scope API

[Dave Chinner <david@fromorbit.com>]

On Mon, Feb 19, 2018 at 08:02:26AM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> While we are at it, remove the flags parameter from iomap_write_begin()
> since we passed AOP_FLAG_NOFS to it.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  fs/iomap.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index afd163586aa0..afaf6ffff34f 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -27,6 +27,7 @@
>  #include <linux/task_io_accounting_ops.h>
>  #include <linux/dax.h>
>  #include <linux/sched/signal.h>
> +#include <linux/sched/mm.h>
>  
>  #include "internal.h"
>  
> @@ -109,19 +110,22 @@ iomap_write_failed(struct inode *inode, loff_t pos, unsigned len)
>  }
>  
>  static int
> -iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags,
> +iomap_write_begin(struct inode *inode, loff_t pos, unsigned len,
>  		struct page **pagep, struct iomap *iomap)
>  {
>  	pgoff_t index = pos >> PAGE_SHIFT;
>  	struct page *page;
>  	int status = 0;
> +	unsigned nofs_flags;
>  
>  	BUG_ON(pos + len > iomap->offset + iomap->length);
>  
>  	if (fatal_signal_pending(current))
>  		return -EINTR;
>  
> -	page = grab_cache_page_write_begin(inode->i_mapping, index, flags);
> +	nofs_flags = memalloc_nofs_save();
> +	page = grab_cache_page_write_begin(inode->i_mapping, index, 0);
> +	memalloc_nofs_restore(nofs_flags);

This is wrong. flags are provided by the caller, so the caller
decides on the memory allocation scope, not this function.

As it is, I can't say I like this whole patchset. This is not what
the scoped memory allocation API was intended for. i.e. it was
intended to mark large regions of memory allocation restrictions for
disjoint operations, not replace a neat, clean flag interface
with a mess of save/restore calls and new ways to screw up error
handling....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com