zfs-patches/0004-Fix-ARC-hit-rate.patch

   1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
   2 From: Brian Behlendorf <behlendorf1@llnl.gov>
   3 Date: Mon, 8 Jan 2018 09:52:36 -0800
   4 Subject: [PATCH] Fix ARC hit rate
   5 MIME-Version: 1.0
   6 Content-Type: text/plain; charset=UTF-8
   7 Content-Transfer-Encoding: 8bit
   8
   9 When the compressed ARC feature was added in commit d3c2ae1
  10 the method of reference counting in the ARC was modified.  As
  11 part of this accounting change the arc_buf_add_ref() function
  12 was removed entirely.
  13
  14 This would have be fine but the arc_buf_add_ref() function
  15 served a second undocumented purpose of updating the ARC access
  16 information when taking a hold on a dbuf.  Without this logic
  17 in place a cached dbuf would not migrate its associated
  18 arc_buf_hdr_t to the MFU list.  This would negatively impact
  19 the ARC hit rate, particularly on systems with a small ARC.
  20
  21 This change reinstates the missing call to arc_access() from
  22 dbuf_hold() by implementing a new arc_buf_access() function.
  23
  24 Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
  25 Reviewed-by: Tony Hutter <hutter2@llnl.gov>
  26 Reviewed-by: Tim Chase <tim@chase2k.com>
  27 Reviewed by: George Wilson <george.wilson@delphix.com>
  28 Reviewed-by: George Melikov <mail@gmelikov.ru>
  29 Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
  30 Closes #6171
  31 Closes #6852
  32 Closes #6989
  33 Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
  34 ---
  35  include/sys/arc.h |  1 +
  36  module/zfs/arc.c  | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
  37  module/zfs/dbuf.c |  4 +++-
  38  3 files changed, 56 insertions(+), 3 deletions(-)
  39
  40 diff --git a/include/sys/arc.h b/include/sys/arc.h
  41 index 66f37cf71..1ea4937bd 100644
  42 --- a/include/sys/arc.h
  43 +++ b/include/sys/arc.h
  44 @@ -221,6 +221,7 @@ void arc_buf_destroy(arc_buf_t *buf, void *tag);
  45  void arc_buf_info(arc_buf_t *buf, arc_buf_info_t *abi, int state_index);
  46  uint64_t arc_buf_size(arc_buf_t *buf);
  47  uint64_t arc_buf_lsize(arc_buf_t *buf);
  48 +void arc_buf_access(arc_buf_t *buf);
  49  void arc_release(arc_buf_t *buf, void *tag);
  50  int arc_released(arc_buf_t *buf);
  51  void arc_buf_sigsegv(int sig, siginfo_t *si, void *unused);
  52 diff --git a/module/zfs/arc.c b/module/zfs/arc.c
  53 index 2b0a78d4b..264e67735 100644
  54 --- a/module/zfs/arc.c
  55 +++ b/module/zfs/arc.c
  56 @@ -429,9 +429,14 @@ typedef struct arc_stats {
  57          * by multiple buffers.
  58          */
  59         kstat_named_t arcstat_mutex_miss;
  60 +       /*
  61 +        * Number of buffers skipped when updating the access state due to the
  62 +        * header having already been released after acquiring the hash lock.
  63 +        */
  64 +       kstat_named_t arcstat_access_skip;
  65         /*
  66          * Number of buffers skipped because they have I/O in progress, are
  67 -        * indrect prefetch buffers that have not lived long enough, or are
  68 +        * indirect prefetch buffers that have not lived long enough, or are
  69          * not from the spa we're trying to evict from.
  70          */
  71         kstat_named_t arcstat_evict_skip;
  72 @@ -667,6 +672,7 @@ static arc_stats_t arc_stats = {
  73         { "mfu_ghost_hits",             KSTAT_DATA_UINT64 },
  74         { "deleted",                    KSTAT_DATA_UINT64 },
  75         { "mutex_miss",                 KSTAT_DATA_UINT64 },
  76 +       { "access_skip",                KSTAT_DATA_UINT64 },
  77         { "evict_skip",                 KSTAT_DATA_UINT64 },
  78         { "evict_not_enough",           KSTAT_DATA_UINT64 },
  79         { "evict_l2_cached",            KSTAT_DATA_UINT64 },
  80 @@ -4926,7 +4932,51 @@ arc_access(arc_buf_hdr_t *hdr, kmutex_t *hash_lock)
  81         }
  82  }
  83
  84 -/* a generic arc_done_func_t which you can use */
  85 +/*
  86 + * This routine is called by dbuf_hold() to update the arc_access() state
  87 + * which otherwise would be skipped for entries in the dbuf cache.
  88 + */
  89 +void
  90 +arc_buf_access(arc_buf_t *buf)
  91 +{
  92 +       mutex_enter(&buf->b_evict_lock);
  93 +       arc_buf_hdr_t *hdr = buf->b_hdr;
  94 +
  95 +       /*
  96 +        * Avoid taking the hash_lock when possible as an optimization.
  97 +        * The header must be checked again under the hash_lock in order
  98 +        * to handle the case where it is concurrently being released.
  99 +        */
 100 +       if (hdr->b_l1hdr.b_state == arc_anon || HDR_EMPTY(hdr)) {
 101 +               mutex_exit(&buf->b_evict_lock);
 102 +               return;
 103 +       }
 104 +
 105 +       kmutex_t *hash_lock = HDR_LOCK(hdr);
 106 +       mutex_enter(hash_lock);
 107 +
 108 +       if (hdr->b_l1hdr.b_state == arc_anon || HDR_EMPTY(hdr)) {
 109 +               mutex_exit(hash_lock);
 110 +               mutex_exit(&buf->b_evict_lock);
 111 +               ARCSTAT_BUMP(arcstat_access_skip);
 112 +               return;
 113 +       }
 114 +
 115 +       mutex_exit(&buf->b_evict_lock);
 116 +
 117 +       ASSERT(hdr->b_l1hdr.b_state == arc_mru ||
 118 +           hdr->b_l1hdr.b_state == arc_mfu);
 119 +
 120 +       DTRACE_PROBE1(arc__hit, arc_buf_hdr_t *, hdr);
 121 +       arc_access(hdr, hash_lock);
 122 +       mutex_exit(hash_lock);
 123 +
 124 +       ARCSTAT_BUMP(arcstat_hits);
 125 +       ARCSTAT_CONDSTAT(!HDR_PREFETCH(hdr), demand, prefetch,
 126 +           !HDR_ISTYPE_METADATA(hdr), data, metadata, hits);
 127 +}
 128 +
 129 +/* a generic arc_read_done_func_t which you can use */
 130  /* ARGSUSED */
 131  void
 132  arc_bcopy_func(zio_t *zio, arc_buf_t *buf, void *arg)
 133 diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c
 134 index 60f52d294..4ee121f5a 100644
 135 --- a/module/zfs/dbuf.c
 136 +++ b/module/zfs/dbuf.c
 137 @@ -2719,8 +2719,10 @@ __dbuf_hold_impl(struct dbuf_hold_impl_data *dh)
 138                 return (SET_ERROR(ENOENT));
 139         }
 140
 141 -       if (dh->dh_db->db_buf != NULL)
 142 +       if (dh->dh_db->db_buf != NULL) {
 143 +               arc_buf_access(dh->dh_db->db_buf);
 144                 ASSERT3P(dh->dh_db->db.db_data, ==, dh->dh_db->db_buf->b_data);
 145 +       }
 146
 147         ASSERT(dh->dh_db->db_buf == NULL || arc_referenced(dh->dh_db->db_buf));
 148
 149 --
 150 2.14.2
 151