Re: [RFC 1/1] fs/reiserfs/journal.c: Remove obsolete __GFP_NOFAIL

tytso
2014-03-26T13:29:49Z

On Tue, Mar 25, 2014 at 11:32:09PM -0700, Andrew Morton wrote:
> Well, there are always alternatives. For example ext3 could
> preallocate a single transaction_t and a single IO page and fall back
> to synchronous page-at-a-time journal writes. But I can totally see
> that such things are unattractive: heaps of new code which is never
> tested in real life. The page allocator works so damn well that it
> doesn't make sense to implement it.

I'm not sure that there are _always_ solutions. For example, ext3
still needs to use buffer cache to do the I/O, and that may require
allocations as well. Fortunately, this was patched around other ways
to avoid livelock issues in the page cleaner in 2013: 84235de394d977

But that's another new user of GFP_NOFAIL (and one added three years
after David tried to declare There Shalt Be No New Users of
GFP_NOFAIL), and sure, we could probably patch around that by having
places where there's no other alternaive to keep a preallocated batch
of pages and/or allocated structures at each code site. But that's as
bad as looping at each code site; in fact, wouldn't it be better if
the allocator wants to avoid looping, to have a separate batch of
pages which (ala GFP_ATOMIC) which is reserved for just for GFP_NOFAIL
allocations when all else fails?

(BTW, we have a similar issue with bio's in the block layer, but
fortunately, those structures are relatively small, and since they are
in their own slab cache, and are constantly being used and then
recycled, it's in practice rare that allocations will fail when we are
in a desparate low memory situation. But technically there will be
places where we should pass a gfp_t down to the block layers, and use
GFP_NOFAIL if we really want to make sure that memory allocations
needed to try to clean pages and/or avoid user data corruption are
needed.)

> Please use NOFAIL ;) The core page allocator will always be able to
> implement this better than callers.

I'll convert jbd2 to use NOFAIL. :-)

Cheers,

- Ted