From owner-svn-src-head@freebsd.org Mon Apr 16 13:59:36 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 113ADF856F1; Mon, 16 Apr 2018 13:59:36 +0000 (UTC) (envelope-from mavbsd@gmail.com) Received: from mail-yw0-x241.google.com (mail-yw0-x241.google.com [IPv6:2607:f8b0:4002:c05::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8A0E9878F4; Mon, 16 Apr 2018 13:59:35 +0000 (UTC) (envelope-from mavbsd@gmail.com) Received: by mail-yw0-x241.google.com with SMTP id q12so6642184ywj.0; Mon, 16 Apr 2018 06:59:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:references:from:openpgp:autocrypt:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=AcZn+xfACg8anY14gdY0BOMrslEVqv7vPKWDBzt76Dc=; b=XWJSUl9RfV7NbM28Bo0NKANjRSLAsbZJWNtjvKJ97vp8B9DdrBwBhj0sbGPUlujcrb TrM4IBDgYpO5/wFbu9DJ1sN7KiHfgKuDhteh6GGlGOil1kZhKoOlQFLeQg5uclut8Pfa +jfG6CFBYXdNslKxkbDFNka7oyFuzb7XGm/u/xYxv5b8eCxpdXi0ob/MgNTbRWuFQAzl qziANC7J8/f5QhAVY5QJWGPE6poaeRfY3mp2TSiTko9JIp9GUScqwf0GwaSs+8qX29K0 Y7QpveubfTh0XjwYY1i8mJM7I+ZTJtCvLpV1jJYWYKYJ+ZoVvnpG34PnK6E4IUyOU0hO /LNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:references:from:openpgp :autocrypt:message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=AcZn+xfACg8anY14gdY0BOMrslEVqv7vPKWDBzt76Dc=; b=RATXgiiv53/tkXx67hiIW63dAyK87JLCYY1x91Ojhv48vLKctPwsFyr601jduPUilB 8HhHPP6t9PJGgmcMF4RsaRU1FdT1nb6YxF9dN/s6PGFFnZsFMPLnVi2sFzylEhoXyqI9 ydbl74kyOcLIl/Xn3EABON5KCTsG39giVZZiPi39uzQT4HEgsFKjToHGPPgZXClfmrhY uX7X9rq5dNjsvLIepM3/20ydKPb5kAn8MV5TVjU4JGDPg35rL70a0ApZEHNE2uJ8YfF/ u2O9uNoIKrwib0X8PbTSVJ/GmxVx9+fSGUO7BEJVBIOvOFzhrYrHSWlnnfSgpMWSXLsd wriw== X-Gm-Message-State: ALQs6tBlZ84Y90h8RDYOeLQUeL4QfNTC5n+XVXnwus0RS7xaU5FRBCC5 6qy5r2+8svsbEAS53Sx/TZCnfs1L X-Google-Smtp-Source: AIpwx48fgMFGHzRMz1WD4EgmWio4IJDQeuqT44Kk/YFkUq9lPvOBq+vfFs4ncYnloadjDMV4P19YcQ== X-Received: by 10.129.51.196 with SMTP id z187mr4261050ywz.512.1523887174733; Mon, 16 Apr 2018 06:59:34 -0700 (PDT) Received: from mavoffice.ixsystems.com ([12.164.17.129]) by smtp.gmail.com with ESMTPSA id p4sm5140018ywb.47.2018.04.16.06.59.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Apr 2018 06:59:34 -0700 (PDT) Sender: Alexander Motin Subject: Re: svn commit: r332523 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys To: Steven Hartland , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201804160054.w3G0swF1030058@repo.freebsd.org> From: Alexander Motin Openpgp: preference=signencrypt Autocrypt: addr=mav@FreeBSD.org; prefer-encrypt=mutual; keydata= xsBNBFOzxAwBCADkPrax0pI2W/ig0CK9nRJJwsHitAGEZ2HZiFEuti+6/4UVxj81yr4ak/4g 9bKUyC7rMEAp/ZHNhd+MFCPAAcHPvtovnfykqE/vuosCS3wlSLloix2iKVLks0CwbLHGAyne 46lTQW74Xl/33c3W1Z6d8jD9gVFT/xaVzZ0U9xdzOmsYAZaAj4ki0tuxO9F7L+ct9grRe7iP g8t9hai7BL4ee3VRwk2JXnKb7UvBiVITKYWKz1jRvZIrjPokgEcCLOSlv7x/1kjuFnj3xWZU 7HSFFT8J93epBbrSSCsYsppIk2fZH41kaaFXsMQfTPH8wkeM6qwrvOh4HiQM08R+9tThABEB AAHNIUFsZXhhbmRlciBNb3RpbiA8bWF2QEZyZWVCU0Qub3JnPsLAlwQTAQoAQQIbAwULCQgH AwUVCgkICwUWAwIBAAIeAQIXgAIZARYhBOmM88TmnMPNDledVYMYw5VbqyJ/BQJZYMKuBQkN McyiAAoJEIMYw5VbqyJ/tuUIAOG3ONOSNYqjK4eTZ1TVh9jdUBAhWk5nhDFnODN49Wj0AbYm 7aIqy8O1hnCDSZG5LttjSAo3UfXJZDKQM0BLb0gpRMBnAYqO6tdolLNqAbPGJBnGoPjsh24y 6KcbDaNnis+lD4GwPXwQM+92wZGhCUFElPV9NciZGVS65TNIgk7X+yEjjhD1MSWKKijZ1r9Z zIt4OzUTxxNOvzdlABZS88nNRdJkatOQJPmFdd1mpP6UzTNCiLUo1pIqOEtJgvVVDYq5WHY6 tciWWYdmZG/tIBexJmv2mV2OLVjXR6ZeKmntVH14H72/wRHJuYHQC+r5SVRcWWayrThsY6jZ Yr4+raTOwE0EU7PEDAEIAOZgWf2cJIu+58IzP2dkXE/urj3tr4OqrB/yHGWUf71Lz6D0Fi6Z AXgDtmcFLGPfMyWuLAvSM+xmoguk7zC4hRBYvQycmIhuqBq1jO1Wp/Z+lpoPM/1cDYLn8Flv mI/c40MhUZh345DA4jYWWaZNjQHUWVQ1fPf595vdVVMPT/abE8E5DaF6fSkRmqFTmfYRkfbt 3ytU8NdUapDcJVY7cEP2nJBVNZPnOIObR/ZIgSxjjrG5o34yXoqeup8JvwEv+/NylzzuyXEZ R1EdEIzQ/a1nh/0j4NXtzZEqKW4aTWlmSqb6wN8jh1OSOOqkYsfnE3nfxcZbxi4IRoNQYlm5 9R8AEQEAAcLAZQQYAQoADwUCU7PEDAIbDAUJBaOagAAKCRCDGMOVW6sif7FRB/4k9y/GaGqU fcJiXdQHRAKHCUvbKMFgeEDHOg33qx+POS2Ah85/PXVa2jYBldCZDmYc+zl48aEMd163a7s3 0gJaB7CYElwxlKUk6c+5gwoYIJuJJzSzW0JzSD5ch7RIRxbfxrKdsiHrUW8AeduZWzlK6VaW RmWILgLmxfLdhEVFWxbr99GSeVFZaZwn6tl/8CvBcgYoARvJvl0V5zS1akQfEISYkwL9EfUI W44EOHranL5qUXkedXBYp6fRsooGrIimfwYxaC8FbXhk3FMgMjDMRiVq4POHo1iGeYETsUrL NM6184E25gPVtX2fb3RhM8Xh6BkwCZ6ZYbQ+AcD4F/cKwsB8BBgBCgAmAhsMFiEE6YzzxOac w80OV51VgxjDlVurIn8FAllgwtgFCQ0xzMwACgkQgxjDlVurIn9OqAf9FAcKWS95wTTbraXA qg/+bQyHgjlMtGCgkmfxLsbUGeqiFgmSIuoDrF7q6sYPs6p00CXXZRuuNZt0lX7O95re8mgz gxm5iJisZpdbHMVepYlw/AxT2wCHwxGCEe64Lm+A9vjlOd+3D3/6fSLwZ9WFCE6p6lQZ1CDg 09xe+JKSgC+KDqmn0tzGKyfSCuhRAq3XkZyxL1hxBaDeP0eeKlzoy7jXodf3wVvXXc0cmpza B5McuRHK4EU6jIioHo30YqPM4AjPHGxV2X1N6/Aayungzj9EXNZtKCxs6dsTvjniWa5VkZ9F 4SOdSbxEen1DZRYpeWnd7GVmO86n+5USkKCXPg== Message-ID: <36d02ae2-0a99-6430-7244-46901268725c@FreeBSD.org> Date: Mon, 16 Apr 2018 09:59:33 -0400 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Apr 2018 13:59:36 -0000 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 >> Reviewed-by: Tony Hutter >> Reviewed-by: Tim Chase >> Reviewed by: George Wilson >> Reviewed-by: George Melikov >> Signed-off-by: Brian Behlendorf >> >> 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