jbd2-get-rid-of-open-coded-allocation-retry-loop

   1 jbd2: get rid of open coded allocation retry loop
   2
   3 From: Michal Hocko <mhocko@suse.cz>
   4
   5 insert_revoke_hash does an open coded endless allocation loop if
   6 journal_oom_retry is true. It doesn't implement any allocation fallback
   7 strategy between the retries, though. The memory allocator doesn't know
   8 about the never fail requirement so it cannot potentially help to move
   9 on with the allocation (e.g. use memory reserves).
  10
  11 Get rid of the retry loop and use __GFP_NOFAIL instead. We will lose the
  12 debugging message but I am not sure it is anyhow helpful.
  13
  14 Do the same for journal_alloc_journal_head which is doing a similar
  15 thing.
  16
  17 Signed-off-by: Michal Hocko <mhocko@suse.cz>
  18 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  19 ---
  20
  21 Hi,
  22 While looking at something unrelated I have encountered the following
  23 two open coded endless loops around allocation request. I have converted
  24 them to use __GFP_NOFAIL in lines of http://marc.info/?l=linux-mm&m=143377014630186&w=2.
  25
  26 journal_oom_retry is hardcoded to 1 and used only at a single place in
  27 jbd2 code so I guess it is just a left over. Nevertheless, I have kept
  28 it there because I do not understand its historical purpose. Anyway
  29 it seems like removing it would allow some more clean ups because
  30 insert_revoke_hash doesn't have any other failure modes (same applies to
  31 jbd2_journal_set_revoke). Let me know if you would be interested in
  32 such a cleanup.
  33
  34 Thanks!
  35
  36  fs/jbd2/journal.c |  6 ++----
  37  fs/jbd2/revoke.c  | 15 +++++----------
  38  2 files changed, 7 insertions(+), 14 deletions(-)
  39
  40 diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
  41 index b96bd8076b70..218414703f11 100644
  42 --- a/fs/jbd2/journal.c
  43 +++ b/fs/jbd2/journal.c
  44 @@ -2362,10 +2362,8 @@ static struct journal_head *journal_alloc_journal_head(void)
  45         if (!ret) {
  46                 jbd_debug(1, "out of memory for journal_head\n");
  47                 pr_notice_ratelimited("ENOMEM in %s, retrying.\n", __func__);
  48 -               while (!ret) {
  49 -                       yield();
  50 -                       ret = kmem_cache_zalloc(jbd2_journal_head_cache, GFP_NOFS);
  51 -               }
  52 +               ret = kmem_cache_zalloc(jbd2_journal_head_cache,
  53 +                               GFP_NOFS | __GFP_NOFAIL);
  54         }
  55         return ret;
  56  }
  57 diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
  58 index c6cbaef2bda1..f9eb93e1b3cb 100644
  59 --- a/fs/jbd2/revoke.c
  60 +++ b/fs/jbd2/revoke.c
  61 @@ -141,11 +141,13 @@ static int insert_revoke_hash(journal_t *journal, unsigned long long blocknr,
  62  {
  63         struct list_head *hash_list;
  64         struct jbd2_revoke_record_s *record;
  65 +       gfp_t gfp_mask = GFP_NOFS;
  66
  67 -repeat:
  68 -       record = kmem_cache_alloc(jbd2_revoke_record_cache, GFP_NOFS);
  69 +       if (journal_oom_retry)
  70 +               gfp_mask |= __GFP_NOFAIL;
  71 +       record = kmem_cache_alloc(jbd2_revoke_record_cache, gfp_mask);
  72         if (!record)
  73 -               goto oom;
  74 +               return -ENOMEM;
  75
  76         record->sequence = seq;
  77         record->blocknr = blocknr;
  78 @@ -154,13 +156,6 @@ static int insert_revoke_hash(journal_t *journal, unsigned long long blocknr,
  79         list_add(&record->hash, hash_list);
  80         spin_unlock(&journal->j_revoke_lock);
  81         return 0;
  82 -
  83 -oom:
  84 -       if (!journal_oom_retry)
  85 -               return -ENOMEM;
  86 -       jbd_debug(1, "ENOMEM in %s, retrying\n", __func__);
  87 -       yield();
  88 -       goto repeat;
  89  }
  90
  91  /* Find a revoke record in the journal's hash table. */
  92 --
  93 2.1.4
  94
  95