linux-kernel - Re: [PATCH 02/12] buffer: grow_dev_page() should use __GFP

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: <20171003155213.GN11879@quack2.suse.cz>
Date:   Tue, 3 Oct 2017 17:52:13 +0200
From:   Jan Kara <jack@...e.cz>
To:     Jens Axboe <axboe@...nel.dk>
Cc:     Jan Kara <jack@...e.cz>, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, hannes@...xchg.org,
        torvalds@...ux-foundation.org
Subject: Re: [PATCH 02/12] buffer: grow_dev_page() should use __GFP_NOFAIL
 for all cases

On Tue 03-10-17 08:36:16, Jens Axboe wrote:
> On 10/03/2017 06:25 AM, Jan Kara wrote:
> > On Tue 03-10-17 14:10:49, Jan Kara wrote:
> >> On Wed 27-09-17 14:13:49, Jens Axboe wrote:
> >>> We currently it it for find_or_create_page(), which means that it
> >>> cannot fail. Ensure we also pass in 'retry == true' to
> >>> alloc_page_buffers(), which also ensure that it cannot fail.
> >>>
> >>> After this, there are no failure cases in grow_dev_page() that
> >>> occur because of a failed memory allocation.
> >>>
> >>> Signed-off-by: Jens Axboe <axboe@...nel.dk>
> >>
> >> Makes sense. You can add:
> >>
> >> Reviewed-by: Jan Kara <jack@...e.cz>
> > 
> > I forgot one question though:
> > 
> >>>  	page = find_or_create_page(inode->i_mapping, index, gfp_mask);
> >>> -	if (!page)
> >>> -		return ret;
> > 
> > Are we sure find_or_create_page() cannot fail for other reasons than
> > ENOMEM? Currently it seems to be the case AFAICT but it isn't obvious to me
> > that is guaranteed in future as well...
> 
> It looks up a page, or allocates and adds one. If we tell the allocator
> that we cannot tolerate failure, then I don't see what else would be
> able to make it fail. That function is such an integral part of
> lookup/create for the page cache.

Well, there memcg stuff in there, radix tree handling etc. So it is not
only an allocation that could fail. But all take care of not failing if
GFP_NOFAIL is set so I guess there are good chances for this to hold even
in the future.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR