Skip to content

Commit

Permalink
Fix ARC hit rate
Browse files Browse the repository at this point in the history
When the compressed ARC feature was added in commit d3c2ae1
the method of reference counting in the ARC was modified.  As
part of this accounting change the arc_buf_add_ref() function
was removed entirely.

This would have be fine but the arc_buf_add_ref() function
served a second undocumented purpose of updating the ARC access
information when taking a hold on a dbuf.  Without this logic
in place a cached dbuf would not migrate its associated
arc_buf_hdr_t to the MFU list.  This would negatively impact
the ARC hit rate, particularly on systems with a small ARC.

This change reinstates the missing call to arc_access() from
dbuf_hold() by implementing a new arc_buf_access() function.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #6171
  • Loading branch information
behlendorf committed Dec 21, 2017
1 parent a8b2e30 commit 48c5f29
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 1 deletion.
1 change: 1 addition & 0 deletions include/sys/arc.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ void arc_buf_destroy(arc_buf_t *buf, void *tag);
void arc_buf_info(arc_buf_t *buf, arc_buf_info_t *abi, int state_index);
uint64_t arc_buf_size(arc_buf_t *buf);
uint64_t arc_buf_lsize(arc_buf_t *buf);
void arc_buf_access(arc_buf_t *buf);
void arc_release(arc_buf_t *buf, void *tag);
int arc_released(arc_buf_t *buf);
void arc_buf_sigsegv(int sig, siginfo_t *si, void *unused);
Expand Down
47 changes: 47 additions & 0 deletions module/zfs/arc.c
Original file line number Diff line number Diff line change
Expand Up @@ -5611,6 +5611,53 @@ arc_access(arc_buf_hdr_t *hdr, kmutex_t *hash_lock)
}
}

/*
* This routine is called by dbuf_hold() to update the arc_access()
* information which otherwise would be skipped for dbufs in the cache.
*/
void
arc_buf_access(arc_buf_t *buf)
{
arc_buf_hdr_t *hdr;
kmutex_t *hash_lock;

mutex_enter(&buf->b_evict_lock);
hdr = buf->b_hdr;

if (hdr->b_l1hdr.b_state == arc_anon || HDR_EMPTY(hdr)) {
mutex_exit(&buf->b_evict_lock);
return;
}

/*
* Because the hash_lock is obtained from the arc_buf_hdr_t it is
* possible the arc_buf_hdr_t may be released before the hash_lock
* can be acquired. Detect this case by verifying the arc_buf_hdr_t
* is still hashed after taking the lock.
*/
hash_lock = HDR_LOCK(buf->b_hdr);
mutex_enter(hash_lock);

if (!HDR_IN_HASH_TABLE(hdr)) {
mutex_exit(hash_lock);
mutex_exit(&buf->b_evict_lock);
return;
}

mutex_exit(&buf->b_evict_lock);

ASSERT(hdr->b_l1hdr.b_state == arc_mru ||
hdr->b_l1hdr.b_state == arc_mfu);

DTRACE_PROBE1(arc__hit, arc_buf_hdr_t *, hdr);
arc_access(hdr, hash_lock);
mutex_exit(hash_lock);

ARCSTAT_BUMP(arcstat_hits);
ARCSTAT_CONDSTAT(!HDR_PREFETCH(hdr) && !HDR_PRESCIENT_PREFETCH(hdr),
demand, prefetch, !HDR_ISTYPE_METADATA(hdr), data, metadata, hits);
}

/* a generic arc_read_done_func_t which you can use */
/* ARGSUSED */
void
Expand Down
4 changes: 3 additions & 1 deletion module/zfs/dbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -2821,8 +2821,10 @@ __dbuf_hold_impl(struct dbuf_hold_impl_data *dh)
return (SET_ERROR(ENOENT));
}

if (dh->dh_db->db_buf != NULL)
if (dh->dh_db->db_buf != NULL) {
arc_buf_access(dh->dh_db->db_buf);
ASSERT3P(dh->dh_db->db.db_data, ==, dh->dh_db->db_buf->b_data);
}

ASSERT(dh->dh_db->db_buf == NULL || arc_referenced(dh->dh_db->db_buf));

Expand Down

0 comments on commit 48c5f29

Please sign in to comment.