Hi Steve, Yes, I am going to merge it in few days. I am trying to merge everything to keep code in sync. I am just rarely tagging ZFS commits with MFC, since it is hard to forget for too long about that exercise. :)
On 16.04.2018 03:36, Steven Hartland wrote: > Hey Mav, this seems like an important one to get in for 11.2 so just > wanted to check if that was your intention as there's no MFC tag on the > commit? > > On 16/04/2018 01:54, Alexander Motin wrote: >> Author: mav >> Date: Mon Apr 16 00:54:58 2018 >> New Revision: 332523 >> URL: https://svnweb.freebsd.org/changeset/base/332523 >> >> Log: >> 9433 Fix ARC hit rate >> >> 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. >> >> Reviewed-by: Giuseppe Di Natale <dinata...@llnl.gov> >> Reviewed-by: Tony Hutter <hutt...@llnl.gov> >> Reviewed-by: Tim Chase <t...@chase2k.com> >> Reviewed by: George Wilson <george.wil...@delphix.com> >> Reviewed-by: George Melikov <m...@gmelikov.ru> >> Signed-off-by: Brian Behlendorf <behlendo...@llnl.gov> >> >> Modified: >> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c >> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c >> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h >> >> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c >> ============================================================================== >> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Mon Apr >> 16 00:42:45 2018 (r332522) >> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Mon Apr >> 16 00:54:58 2018 (r332523) >> @@ -540,8 +540,13 @@ typedef struct arc_stats { >> */ >> kstat_named_t arcstat_mutex_miss; >> /* >> + * Number of buffers skipped when updating the access state due to the >> + * header having already been released after acquiring the hash lock. >> + */ >> + kstat_named_t arcstat_access_skip; >> + /* >> * Number of buffers skipped because they have I/O in progress, are >> - * indrect prefetch buffers that have not lived long enough, or are >> + * indirect prefetch buffers that have not lived long enough, or are >> * not from the spa we're trying to evict from. >> */ >> kstat_named_t arcstat_evict_skip; >> @@ -796,6 +801,7 @@ static arc_stats_t arc_stats = { >> { "allocated", KSTAT_DATA_UINT64 }, >> { "deleted", KSTAT_DATA_UINT64 }, >> { "mutex_miss", KSTAT_DATA_UINT64 }, >> + { "access_skip", KSTAT_DATA_UINT64 }, >> { "evict_skip", KSTAT_DATA_UINT64 }, >> { "evict_not_enough", KSTAT_DATA_UINT64 }, >> { "evict_l2_cached", KSTAT_DATA_UINT64 }, >> @@ -5063,6 +5069,51 @@ arc_access(arc_buf_hdr_t *hdr, kmutex_t *hash_lock) >> } else { >> ASSERT(!"invalid arc state"); >> } >> +} >> + >> +/* >> + * This routine is called by dbuf_hold() to update the arc_access() state >> + * which otherwise would be skipped for entries in the dbuf cache. >> + */ >> +void >> +arc_buf_access(arc_buf_t *buf) >> +{ >> + mutex_enter(&buf->b_evict_lock); >> + arc_buf_hdr_t *hdr = buf->b_hdr; >> + >> + /* >> + * Avoid taking the hash_lock when possible as an optimization. >> + * The header must be checked again under the hash_lock in order >> + * to handle the case where it is concurrently being released. >> + */ >> + if (hdr->b_l1hdr.b_state == arc_anon || HDR_EMPTY(hdr)) { >> + mutex_exit(&buf->b_evict_lock); >> + ARCSTAT_BUMP(arcstat_access_skip); >> + return; >> + } >> + >> + kmutex_t *hash_lock = HDR_LOCK(hdr); >> + mutex_enter(hash_lock); >> + >> + if (hdr->b_l1hdr.b_state == arc_anon || HDR_EMPTY(hdr)) { >> + mutex_exit(hash_lock); >> + mutex_exit(&buf->b_evict_lock); >> + ARCSTAT_BUMP(arcstat_access_skip); >> + 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), >> + demand, prefetch, !HDR_ISTYPE_METADATA(hdr), data, metadata, hits); >> } >> >> /* a generic arc_done_func_t which you can use */ >> >> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c >> ============================================================================== >> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c Mon Apr >> 16 00:42:45 2018 (r332522) >> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c Mon Apr >> 16 00:54:58 2018 (r332523) >> @@ -2579,8 +2579,10 @@ top: >> return (SET_ERROR(ENOENT)); >> } >> >> - if (db->db_buf != NULL) >> + if (db->db_buf != NULL) { >> + arc_buf_access(db->db_buf); >> ASSERT3P(db->db.db_data, ==, db->db_buf->b_data); >> + } >> >> ASSERT(db->db_buf == NULL || arc_referenced(db->db_buf)); >> >> >> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h >> ============================================================================== >> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h Mon Apr >> 16 00:42:45 2018 (r332522) >> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/arc.h Mon Apr >> 16 00:54:58 2018 (r332523) >> @@ -169,6 +169,7 @@ void arc_loan_inuse_buf(arc_buf_t *buf, void *tag); >> void arc_buf_destroy(arc_buf_t *buf, void *tag); >> int arc_buf_size(arc_buf_t *buf); >> int 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_freeze(arc_buf_t *buf); >> > -- Alexander Motin _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
