svn commit: r332523 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys
Alexander Motin
mav at FreeBSD.org
Mon Apr 16 13:59:36 UTC 2018
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 <dinatale2 at llnl.gov>
>> Reviewed-by: Tony Hutter <hutter2 at llnl.gov>
>> Reviewed-by: Tim Chase <tim at chase2k.com>
>> Reviewed by: George Wilson <george.wilson at delphix.com>
>> Reviewed-by: George Melikov <mail at gmelikov.ru>
>> Signed-off-by: Brian Behlendorf <behlendorf1 at 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
More information about the svn-src-all
mailing list