Skip to content

Commit

Permalink
Fix/improve dbuf hits accounting
Browse files Browse the repository at this point in the history 
Instead of clearing stats inside arc_buf_alloc_impl() do it inside
arc_hdr_alloc() and arc_release().  It fixes statistics being wiped
every time a new dbuf is filled from the ARC.

Remove b_l1hdr.b_l2_hits. L2ARC hits are accounted at b_l2hdr.b_hits.
Since the hits are accounted under hash lock, replace atomics with
simple increments.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: George Wilson <george.wilson@delphix.com>
Signed-off-by: Alexander Motin <mav@FreeBSD.org>
Closes #12422
  • Loading branch information
@amotin @behlendorf
amotin authored and behlendorf committed Aug 31, 2021
1 parent c066ab1 commit 772f58c
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 28 deletions.
4 changes: 2 additions & 2 deletions include/os/linux/zfs/sys/trace_arc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ DECLARE_EVENT_CLASS(zfs_arc_buf_hdr_class,
__entry->hdr_mru_ghost_hits = ab->b_l1hdr.b_mru_ghost_hits;
__entry->hdr_mfu_hits = ab->b_l1hdr.b_mfu_hits;
__entry->hdr_mfu_ghost_hits = ab->b_l1hdr.b_mfu_ghost_hits;
__entry->hdr_l2_hits = ab->b_l1hdr.b_l2_hits;
__entry->hdr_l2_hits = ab->b_l2hdr.b_hits;
__entry->hdr_refcount = ab->b_l1hdr.b_refcnt.rc_count;
),
TP_printk("hdr { dva 0x%llx:0x%llx birth %llu "
Expand Down Expand Up @@ -238,7 +238,7 @@ DECLARE_EVENT_CLASS(zfs_arc_miss_class,
__entry->hdr_mru_ghost_hits = hdr->b_l1hdr.b_mru_ghost_hits;
__entry->hdr_mfu_hits = hdr->b_l1hdr.b_mfu_hits;
__entry->hdr_mfu_ghost_hits = hdr->b_l1hdr.b_mfu_ghost_hits;
__entry->hdr_l2_hits = hdr->b_l1hdr.b_l2_hits;
__entry->hdr_l2_hits = hdr->b_l2hdr.b_hits;
__entry->hdr_refcount = hdr->b_l1hdr.b_refcnt.rc_count;

__entry->bp_dva0[0] = bp->blk_dva[0].dva_word[0];
Expand Down
10 changes: 4 additions & 6 deletions include/sys/arc_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,24 +153,22 @@ typedef struct l1arc_buf_hdr {
kmutex_t b_freeze_lock;
zio_cksum_t *b_freeze_cksum;

arc_buf_t *b_buf;
uint32_t b_bufcnt;
/* for waiting on writes to complete */
/* for waiting on reads to complete */
kcondvar_t b_cv;
uint8_t b_byteswap;


/* protected by arc state mutex */
arc_state_t *b_state;
multilist_node_t b_arc_node;

/* updated atomically */
/* protected by hash lock */
clock_t b_arc_access;
uint32_t b_mru_hits;
uint32_t b_mru_ghost_hits;
uint32_t b_mfu_hits;
uint32_t b_mfu_ghost_hits;
uint32_t b_l2_hits;
uint32_t b_bufcnt;
arc_buf_t *b_buf;

/* self protecting */
zfs_refcount_t b_refcnt;
Expand Down
30 changes: 10 additions & 20 deletions module/zfs/arc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2741,12 +2741,6 @@ arc_buf_alloc_impl(arc_buf_hdr_t *hdr, spa_t *spa, const zbookmark_phys_t *zb,
ASSERT3P(*ret, ==, NULL);
IMPLY(encrypted, compressed);

hdr->b_l1hdr.b_mru_hits = 0;
hdr->b_l1hdr.b_mru_ghost_hits = 0;
hdr->b_l1hdr.b_mfu_hits = 0;
hdr->b_l1hdr.b_mfu_ghost_hits = 0;
hdr->b_l1hdr.b_l2_hits = 0;

buf = *ret = kmem_cache_alloc(buf_cache, KM_PUSHPAGE);
buf->b_hdr = hdr;
buf->b_data = NULL;
Expand Down Expand Up @@ -3278,6 +3272,10 @@ arc_hdr_alloc(uint64_t spa, int32_t psize, int32_t lsize,

hdr->b_l1hdr.b_state = arc_anon;
hdr->b_l1hdr.b_arc_access = 0;
hdr->b_l1hdr.b_mru_hits = 0;
hdr->b_l1hdr.b_mru_ghost_hits = 0;
hdr->b_l1hdr.b_mfu_hits = 0;
hdr->b_l1hdr.b_mfu_ghost_hits = 0;
hdr->b_l1hdr.b_bufcnt = 0;
hdr->b_l1hdr.b_buf = NULL;

Expand Down Expand Up @@ -3466,7 +3464,6 @@ arc_hdr_realloc_crypt(arc_buf_hdr_t *hdr, boolean_t need_crypt)
nhdr->b_l1hdr.b_mru_ghost_hits = hdr->b_l1hdr.b_mru_ghost_hits;
nhdr->b_l1hdr.b_mfu_hits = hdr->b_l1hdr.b_mfu_hits;
nhdr->b_l1hdr.b_mfu_ghost_hits = hdr->b_l1hdr.b_mfu_ghost_hits;
nhdr->b_l1hdr.b_l2_hits = hdr->b_l1hdr.b_l2_hits;
nhdr->b_l1hdr.b_acb = hdr->b_l1hdr.b_acb;
nhdr->b_l1hdr.b_pabd = hdr->b_l1hdr.b_pabd;

Expand Down Expand Up @@ -3511,7 +3508,6 @@ arc_hdr_realloc_crypt(arc_buf_hdr_t *hdr, boolean_t need_crypt)
hdr->b_l1hdr.b_mru_ghost_hits = 0;
hdr->b_l1hdr.b_mfu_hits = 0;
hdr->b_l1hdr.b_mfu_ghost_hits = 0;
hdr->b_l1hdr.b_l2_hits = 0;
hdr->b_l1hdr.b_acb = NULL;
hdr->b_l1hdr.b_pabd = NULL;

Expand Down Expand Up @@ -5433,7 +5429,7 @@ arc_access(arc_buf_hdr_t *hdr, kmutex_t *hash_lock)
arc_hdr_clear_flags(hdr,
ARC_FLAG_PREFETCH |
ARC_FLAG_PRESCIENT_PREFETCH);
atomic_inc_32(&hdr->b_l1hdr.b_mru_hits);
hdr->b_l1hdr.b_mru_hits++;
ARCSTAT_BUMP(arcstat_mru_hits);
if (HDR_HAS_L2HDR(hdr))
l2arc_hdr_arcstats_increment_state(hdr);
Expand All @@ -5458,7 +5454,7 @@ arc_access(arc_buf_hdr_t *hdr, kmutex_t *hash_lock)
DTRACE_PROBE1(new_state__mfu, arc_buf_hdr_t *, hdr);
arc_change_state(arc_mfu, hdr, hash_lock);
}
atomic_inc_32(&hdr->b_l1hdr.b_mru_hits);
hdr->b_l1hdr.b_mru_hits++;
ARCSTAT_BUMP(arcstat_mru_hits);
} else if (hdr->b_l1hdr.b_state == arc_mru_ghost) {
arc_state_t *new_state;
Expand Down Expand Up @@ -5487,7 +5483,7 @@ arc_access(arc_buf_hdr_t *hdr, kmutex_t *hash_lock)
hdr->b_l1hdr.b_arc_access = ddi_get_lbolt();
arc_change_state(new_state, hdr, hash_lock);

atomic_inc_32(&hdr->b_l1hdr.b_mru_ghost_hits);
hdr->b_l1hdr.b_mru_ghost_hits++;
ARCSTAT_BUMP(arcstat_mru_ghost_hits);
} else if (hdr->b_l1hdr.b_state == arc_mfu) {
/*
Expand All @@ -5500,7 +5496,7 @@ arc_access(arc_buf_hdr_t *hdr, kmutex_t *hash_lock)
* the head of the list now.
*/

atomic_inc_32(&hdr->b_l1hdr.b_mfu_hits);
hdr->b_l1hdr.b_mfu_hits++;
ARCSTAT_BUMP(arcstat_mfu_hits);
hdr->b_l1hdr.b_arc_access = ddi_get_lbolt();
} else if (hdr->b_l1hdr.b_state == arc_mfu_ghost) {
Expand All @@ -5523,7 +5519,7 @@ arc_access(arc_buf_hdr_t *hdr, kmutex_t *hash_lock)
DTRACE_PROBE1(new_state__mfu, arc_buf_hdr_t *, hdr);
arc_change_state(new_state, hdr, hash_lock);

atomic_inc_32(&hdr->b_l1hdr.b_mfu_ghost_hits);
hdr->b_l1hdr.b_mfu_ghost_hits++;
ARCSTAT_BUMP(arcstat_mfu_ghost_hits);
} else if (hdr->b_l1hdr.b_state == arc_l2c_only) {
/*
Expand Down Expand Up @@ -6294,7 +6290,7 @@ arc_read(zio_t *pio, spa_t *spa, const blkptr_t *bp,

DTRACE_PROBE1(l2arc__hit, arc_buf_hdr_t *, hdr);
ARCSTAT_BUMP(arcstat_l2_hits);
atomic_inc_32(&hdr->b_l2hdr.b_hits);
hdr->b_l2hdr.b_hits++;

cb = kmem_zalloc(sizeof (l2arc_read_callback_t),
KM_SLEEP);
Expand Down Expand Up @@ -6701,11 +6697,6 @@ arc_release(arc_buf_t *buf, void *tag)
nhdr->b_l1hdr.b_bufcnt = 1;
if (ARC_BUF_ENCRYPTED(buf))
nhdr->b_crypt_hdr.b_ebufcnt = 1;
nhdr->b_l1hdr.b_mru_hits = 0;
nhdr->b_l1hdr.b_mru_ghost_hits = 0;
nhdr->b_l1hdr.b_mfu_hits = 0;
nhdr->b_l1hdr.b_mfu_ghost_hits = 0;
nhdr->b_l1hdr.b_l2_hits = 0;
(void) zfs_refcount_add(&nhdr->b_l1hdr.b_refcnt, tag);
buf->b_hdr = nhdr;

Expand All @@ -6722,7 +6713,6 @@ arc_release(arc_buf_t *buf, void *tag)
hdr->b_l1hdr.b_mru_ghost_hits = 0;
hdr->b_l1hdr.b_mfu_hits = 0;
hdr->b_l1hdr.b_mfu_ghost_hits = 0;
hdr->b_l1hdr.b_l2_hits = 0;
arc_change_state(arc_anon, hdr, hash_lock);
hdr->b_l1hdr.b_arc_access = 0;

Expand Down

0 comments on commit 772f58c

Please sign in to comment.