From: Jan Kara <jack@suse.cz>
To: Jens Axboe <axboe@kernel.dk>
Cc: Jan Kara <jack@suse.cz>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
hannes@cmpxchg.org, torvalds@linux-foundation.org
Subject: Re: [PATCH 02/12] buffer: grow_dev_page() should use __GFP_NOFAIL for all cases
Date: Tue, 3 Oct 2017 17:52:13 +0200 [thread overview]
Message-ID: <20171003155213.GN11879@quack2.suse.cz> (raw)
In-Reply-To: <ee1b4215-2846-841f-9f05-6a66e3e7665a@kernel.dk>
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@kernel.dk>
> >>
> >> Makes sense. You can add:
> >>
> >> Reviewed-by: Jan Kara <jack@suse.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@suse.com>
SUSE Labs, CR
next prev parent reply other threads:[~2017-10-03 15:52 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-27 20:13 [PATCH 0/12 v3] Writeback improvements Jens Axboe
2017-09-27 20:13 ` [PATCH 01/12] buffer: have alloc_page_buffers() use __GFP_NOFAIL Jens Axboe
2017-09-28 14:08 ` Nikolay Borisov
2017-10-02 15:02 ` Jan Kara
2017-09-27 20:13 ` [PATCH 02/12] buffer: grow_dev_page() should use __GFP_NOFAIL for all cases Jens Axboe
2017-09-28 14:11 ` Nikolay Borisov
2017-09-28 18:12 ` Jens Axboe
2017-10-03 12:10 ` Jan Kara
2017-10-03 12:25 ` Jan Kara
2017-10-03 14:36 ` Jens Axboe
2017-10-03 15:52 ` Jan Kara [this message]
2017-09-27 20:13 ` [PATCH 03/12] buffer: eliminate the need to call free_more_memory() in __getblk_slow() Jens Axboe
2017-09-28 14:12 ` Nikolay Borisov
2017-10-03 12:22 ` Jan Kara
2017-09-27 20:13 ` [PATCH 04/12] fs: kill 'nr_pages' argument from wakeup_flusher_threads() Jens Axboe
2017-09-27 20:13 ` [PATCH 05/12] writeback: switch wakeup_flusher_threads() to cyclic writeback Jens Axboe
2017-10-03 12:43 ` Jan Kara
2017-09-27 20:13 ` [PATCH 06/12] writeback: provide a wakeup_flusher_threads_bdi() Jens Axboe
2017-09-27 20:13 ` [PATCH 07/12] writeback: pass in '0' for nr_pages writeback in laptop mode Jens Axboe
2017-09-27 20:13 ` [PATCH 08/12] writeback: make wb_start_writeback() static Jens Axboe
2017-09-27 20:13 ` [PATCH 09/12] writeback: move nr_pages == 0 logic to one location Jens Axboe
2017-09-27 20:13 ` [PATCH 10/12] writeback: only allow one inflight and pending full flush Jens Axboe
2017-09-28 21:41 ` Andrew Morton
2017-09-28 21:44 ` Linus Torvalds
2017-09-29 0:17 ` Jens Axboe
2017-09-29 5:21 ` Amir Goldstein
2017-10-03 16:06 ` Matthew Wilcox
2017-10-03 16:11 ` Jens Axboe
2017-10-03 17:03 ` Matthew Wilcox
2017-09-29 0:15 ` Jens Axboe
2017-09-27 20:13 ` [PATCH 11/12] writeback: make sync_inodes_sb() use range cyclic writeback Jens Axboe
2017-09-27 20:13 ` [PATCH 12/12] writeback: kill off ->range_cycle option Jens Axboe
2017-09-27 20:22 ` Jens Axboe
2017-09-28 13:19 ` [PATCH 0/12 v3] Writeback improvements John Stoffel
2017-09-28 13:39 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171003155213.GN11879@quack2.suse.cz \
--to=jack@suse.cz \
--cc=axboe@kernel.dk \
--cc=hannes@cmpxchg.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®