linux-kernel - Re: [PATCH 1/2] etrfs: fix up misleading GFP_NOFS usage in btrfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170111135550.GJ12081@twin.jikos.cz>
Date:   Wed, 11 Jan 2017 14:55:50 +0100
From:   David Sterba <dsterba@...e.cz>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>,
        Josef Bacik <jbacik@...com>, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, Michal Hocko <mhocko@...e.com>
Subject: Re: [PATCH 1/2] etrfs: fix up misleading GFP_NOFS usage in
 btrfs_releasepage

On Mon, Jan 09, 2017 at 03:39:02PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@...e.com>
> 
> b335b0034e25 ("Btrfs: Avoid using __GFP_HIGHMEM with slab allocator")
> has reduced the allocation mask in btrfs_releasepage to GFP_NOFS just
> to prevent from giving an unappropriate gfp mask to the slab allocator
> deeper down the callchain (in alloc_extent_state). This is wrong for
> two reasons a) GFP_NOFS might be just too restrictive for the calling
> context b) it is better to tweak the gfp mask down when it needs that.
> 
> So just remove the mask tweaking from btrfs_releasepage and move it
> down to alloc_extent_state where it is needed.
> 
> Signed-off-by: Michal Hocko <mhocko@...e.com>
> ---
>  fs/btrfs/extent_io.c | 5 +++++
>  fs/btrfs/inode.c     | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index b38150eec6b4..f6ae94a4acad 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -226,6 +226,11 @@ static struct extent_state *alloc_extent_state(gfp_t mask)
>  {
>  	struct extent_state *state;
>  
> +	/*
> +	 * The given mask might be not appropriate for the slab allocator,
> +	 * drop the unsupported bits
> +	 */
> +	mask &= ~(__GFP_DMA32|__GFP_HIGHMEM);

Is this future proof enough? As it's enumerating some gfp flags, what if
more are necessary in the future? I'm interested about some synthetic
gfp flags that would not require knowledge about what is or is not
acceptable for slab allocator.

But otherwise looks ok to me, I'm going to merge the patch. Thanks.