From cfe8e960f193d69a87693474de666025736f16aa Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Tue, 17 Aug 2021 11:50:31 -0400 Subject: [PATCH] Fix/improve dbuf hits accounting 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 Reviewed-by: George Wilson Signed-off-by: Alexander Motin Sponsored-By: iXsystems, Inc. Closes #12422 --- include/os/linux/zfs/sys/trace_arc.h | 4 ++-- include/sys/arc_impl.h | 10 ++++------ module/zfs/arc.c | 30 ++++++++++------------------ 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/include/os/linux/zfs/sys/trace_arc.h b/include/os/linux/zfs/sys/trace_arc.h index 3df491f8b3..d3410bc07a 100644 --- a/include/os/linux/zfs/sys/trace_arc.h +++ b/include/os/linux/zfs/sys/trace_arc.h @@ -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 " @@ -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]; diff --git a/include/sys/arc_impl.h b/include/sys/arc_impl.h index 89be78ce21..8421903fb8 100644 --- a/include/sys/arc_impl.h +++ b/include/sys/arc_impl.h @@ -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; diff --git a/module/zfs/arc.c b/module/zfs/arc.c index a8404182c8..02e181e50d 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -2740,12 +2740,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; @@ -3277,6 +3271,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; @@ -3460,7 +3458,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; @@ -3505,7 +3502,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; @@ -5427,7 +5423,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); @@ -5452,7 +5448,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; @@ -5481,7 +5477,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) { /* @@ -5494,7 +5490,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) { @@ -5517,7 +5513,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) { /* @@ -6288,7 +6284,7 @@ top: 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); @@ -6695,11 +6691,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; @@ -6716,7 +6707,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;