On Fri, 21 Mar 2014 13:00:55 -0700
Andrew Morton <a...@linux-foundation.org> wrote:
> On Fri, 21 Mar 2014 17:18:30 +0100 Fabian Frederick <f...@skynet.be> wrote:
>
> > Loop around congestion_wait on allocation failure/alloc_journal_list
> > like already fixed in other FS.
> >
> > ...
> >
> > --- a/fs/reiserfs/journal.c
> > +++ b/fs/reiserfs/journal.c
> > @@ -2487,8 +2487,13 @@ static int journal_read(struct super_block *sb)
> > static struct reiserfs_journal_list *alloc_journal_list(struct super_block
> > *s)
> > {
> > struct reiserfs_journal_list *jl;
> > - jl = kzalloc(sizeof(struct reiserfs_journal_list),
> > - GFP_NOFS | __GFP_NOFAIL);
> > +
> > + do {
> > + jl = kzalloc(sizeof(struct reiserfs_journal_list), GFP_NOFS);
> > + if (unlikely(!jl))
> > + congestion_wait(BLK_RW_ASYNC, HZ/50);
> > + } while (!jl)
> > +
>
> Dammit, who has been running around converting __GFP_NOFAIL into
> open-coded congestion_wait() loops?
>
> The whole point of __GFP_NOFAIL is to centralise this
> wait-for-memory-for-ever operation. So it is implemented in a common
> (core) place and so that we can easily locate these problematic
> callers.
>
> This comment in ext4:
>
> /*
> * If __GFP_FS is not present, then we may be
> * being called from inside the fs writeback
> * layer, so we MUST NOT fail. Since
> * __GFP_NOFAIL is going away, we will arrange
> * to retry the allocation ourselves.
> */
>
> is exactly wrong. Yes, we'd like __GFP_NOFAIL to go away, but it
> cannot go away until buggy callsites such as this one are *fixed*.
> Removing the __GFP_NOFAIL usage simply hides the buggy code from casual
> searchers.
>
> argh.
>
> What we should do is to fix all these call sites so they can handle
> memory exhaustion. That's hard so in the interim they should be using
> __GFP_NOFAIL.
>
Ok, if even ext4 comments are wrong, things gonna be very difficult :)
Any sample of a callsite transition done well ? (git id ?)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
