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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 19, 2018 at 08:02:26AM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
> 
> 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@xxxxxxxx>
> ---
>  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@xxxxxxxxxxxxx



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux