From owner-dev-commits-src-main@freebsd.org Thu Apr 1 09:11:44 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id BEEB65C590C; Thu, 1 Apr 2021 09:11:44 +0000 (UTC) (envelope-from michael.tuexen@macmic.franken.de) Received: from drew.franken.de (mail-n.franken.de [193.175.24.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.franken.de", Issuer "Sectigo RSA Domain Validation Secure Server CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4F9y9X4G93z3F76; Thu, 1 Apr 2021 09:11:44 +0000 (UTC) (envelope-from michael.tuexen@macmic.franken.de) Received: from [IPv6:2a02:8109:1140:c3d:5411:2b1c:5f67:463d] (unknown [IPv6:2a02:8109:1140:c3d:5411:2b1c:5f67:463d]) (Authenticated sender: macmic) by mail-n.franken.de (Postfix) with ESMTPSA id 4E2AC782946C6; Thu, 1 Apr 2021 11:11:40 +0200 (CEST) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.60.0.2.21\)) Subject: Re: git: 529a2a0f2765 - main - tcp: For hostcache performance, use atomics instead of counters From: Michael Tuexen In-Reply-To: <20210401102035.19636bea@x23> Date: Thu, 1 Apr 2021 11:11:39 +0200 Cc: Richard Scheffenegger , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <9F675C09-14B6-4117-A079-90068E1D1E65@macmic.franken.de> References: <202104010803.13183vQe043756@gitrepo.freebsd.org> <20210401102035.19636bea@x23> To: Marko Zec X-Mailer: Apple Mail (2.3654.60.0.2.21) X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=disabled version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on mail-n.franken.de X-Rspamd-Queue-Id: 4F9y9X4G93z3F76 X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Apr 2021 09:11:44 -0000 > On 1. Apr 2021, at 10:20, Marko Zec wrote: >=20 > On Thu, 1 Apr 2021 08:03:57 GMT > Richard Scheffenegger wrote: >=20 >> The branch main has been updated by rscheff: >>=20 >> URL: >> = https://cgit.FreeBSD.org/src/commit/?id=3D529a2a0f2765f6c57c50a5af6be242c0= 3bf714e3 >>=20 >> commit 529a2a0f2765f6c57c50a5af6be242c03bf714e3 >> Author: Richard Scheffenegger >> AuthorDate: 2021-04-01 08:00:32 +0000 >> Commit: Richard Scheffenegger >> CommitDate: 2021-04-01 08:03:30 +0000 >>=20 >> tcp: For hostcache performance, use atomics instead of counters >=20 > Is that an April 1st joke, or for real, since it appears to have hit > the tree? >=20 >> As accessing the tcp hostcache happens frequently on some >> classes of servers, it was recommended >=20 > Recommended by whom? See https://reviews.freebsd.org/D29510#661680 Best regards Michael >=20 >> to use atomic_add/subtract >> rather than (per-CPU distributed) counters, which have to be >> summed up at high cost to cache efficiency. >=20 > Numbers? >=20 >>=20 >> PR: 254333 >> MFC after: 2 weeks >> Sponsored by: NetApp, Inc. >> Reviewed By: #transport, tuexen, jtl >> Differential Revision: https://reviews.freebsd.org/D29522 >> --- >> sys/netinet/tcp_hostcache.c | 24 +++++++++++------------- >> sys/netinet/tcp_hostcache.h | 2 +- >> 2 files changed, 12 insertions(+), 14 deletions(-) >>=20 >> diff --git a/sys/netinet/tcp_hostcache.c = b/sys/netinet/tcp_hostcache.c >> index 67da97405c3f..87023cc1a760 100644 >> --- a/sys/netinet/tcp_hostcache.c >> +++ b/sys/netinet/tcp_hostcache.c >> @@ -147,8 +147,8 @@ SYSCTL_UINT(_net_inet_tcp_hostcache, OID_AUTO, >> bucketlimit, CTLFLAG_VNET | CTLFLAG_RDTUN, >> &VNET_NAME(tcp_hostcache.bucket_limit), 0, "Per-bucket hash limit for >> hostcache");=20 >> -SYSCTL_COUNTER_U64(_net_inet_tcp_hostcache, OID_AUTO, count, >> CTLFLAG_VNET | CTLFLAG_RD, >> - &VNET_NAME(tcp_hostcache.cache_count), >> +SYSCTL_UINT(_net_inet_tcp_hostcache, OID_AUTO, count, CTLFLAG_VNET | >> CTLFLAG_RD, >> + &VNET_NAME(tcp_hostcache.cache_count), 0, >> "Current number of entries in hostcache"); >>=20 >> SYSCTL_INT(_net_inet_tcp_hostcache, OID_AUTO, expire, CTLFLAG_VNET | >> CTLFLAG_RW, @@ -199,8 +199,7 @@ tcp_hc_init(void) >> /* >> * Initialize hostcache structures. >> */ >> - V_tcp_hostcache.cache_count =3D counter_u64_alloc(M_WAITOK); >> - counter_u64_zero(V_tcp_hostcache.cache_count); >> + atomic_store_int(&V_tcp_hostcache.cache_count, 0); >> V_tcp_hostcache.hashsize =3D TCP_HOSTCACHE_HASHSIZE; >> V_tcp_hostcache.bucket_limit =3D TCP_HOSTCACHE_BUCKETLIMIT; >> V_tcp_hostcache.expire =3D TCP_HOSTCACHE_EXPIRE; >> @@ -268,9 +267,6 @@ tcp_hc_destroy(void) >> /* Purge all hc entries. */ >> tcp_hc_purge_internal(1); >>=20 >> - /* Release the counter */ >> - counter_u64_free(V_tcp_hostcache.cache_count); >> - >> /* Free the uma zone and the allocated hash table. */ >> uma_zdestroy(V_tcp_hostcache.zone); >>=20 >> @@ -378,7 +374,7 @@ tcp_hc_insert(struct in_conninfo *inc) >> * If the bucket limit is reached, reuse the least-used >> element. */ >> if (hc_head->hch_length >=3D V_tcp_hostcache.bucket_limit || >> - counter_u64_fetch(V_tcp_hostcache.cache_count) >=3D >> V_tcp_hostcache.cache_limit) { >> + atomic_load_int(&V_tcp_hostcache.cache_count) >=3D >> V_tcp_hostcache.cache_limit) { hc_entry =3D >> TAILQ_LAST(&hc_head->hch_bucket, hc_qhead); /* >> * At first we were dropping the last element, just >> to @@ -395,7 +391,7 @@ tcp_hc_insert(struct in_conninfo *inc) >> } >> TAILQ_REMOVE(&hc_head->hch_bucket, hc_entry, rmx_q); >> V_tcp_hostcache.hashbase[hash].hch_length--; >> - counter_u64_add(V_tcp_hostcache.cache_count, -1); >> + atomic_subtract_int(&V_tcp_hostcache.cache_count, 1); >> TCPSTAT_INC(tcps_hc_bucketoverflow); >> #if 0 >> uma_zfree(V_tcp_hostcache.zone, hc_entry); >> @@ -428,7 +424,7 @@ tcp_hc_insert(struct in_conninfo *inc) >> */ >> TAILQ_INSERT_HEAD(&hc_head->hch_bucket, hc_entry, rmx_q); >> V_tcp_hostcache.hashbase[hash].hch_length++; >> - counter_u64_add(V_tcp_hostcache.cache_count, 1); >> + atomic_add_int(&V_tcp_hostcache.cache_count, 1); >> TCPSTAT_INC(tcps_hc_added); >>=20 >> return hc_entry; >> @@ -644,7 +640,8 @@ sysctl_tcp_hc_list(SYSCTL_HANDLER_ARGS) >>=20 >> /* Optimize Buffer length query by sbin/sysctl */ >> if (req->oldptr =3D=3D NULL) { >> - len =3D >> (counter_u64_fetch(V_tcp_hostcache.cache_count) + 1) * linesize; >> + len =3D (atomic_load_int(&V_tcp_hostcache.cache_count) >> + 1) * >> + linesize; >> return (SYSCTL_OUT(req, NULL, len)); >> } >>=20 >> @@ -654,7 +651,8 @@ sysctl_tcp_hc_list(SYSCTL_HANDLER_ARGS) >> } >>=20 >> /* Use a buffer sized for one full bucket */ >> - sbuf_new_for_sysctl(&sb, NULL, V_tcp_hostcache.bucket_limit >> * linesize, req); >> + sbuf_new_for_sysctl(&sb, NULL, V_tcp_hostcache.bucket_limit * >> + linesize, req); >>=20 >> sbuf_printf(&sb, >> "\nIP address MTU SSTRESH RTT RTTVAR " >> @@ -716,7 +714,7 @@ tcp_hc_purge_internal(int all) >> hc_entry, rmx_q); >> uma_zfree(V_tcp_hostcache.zone, >> hc_entry); V_tcp_hostcache.hashbase[i].hch_length--; >> - >> counter_u64_add(V_tcp_hostcache.cache_count, -1); >> + >> atomic_subtract_int(&V_tcp_hostcache.cache_count, 1); } else >> hc_entry->rmx_expire -=3D >> V_tcp_hostcache.prune; } >> diff --git a/sys/netinet/tcp_hostcache.h = b/sys/netinet/tcp_hostcache.h >> index b5237392acc2..2f7035c0c6af 100644 >> --- a/sys/netinet/tcp_hostcache.h >> +++ b/sys/netinet/tcp_hostcache.h >> @@ -74,7 +74,7 @@ struct tcp_hostcache { >> u_int hashsize; >> u_int hashmask; >> u_int bucket_limit; >> - counter_u64_t cache_count; >> + u_int cache_count; >> u_int cache_limit; >> int expire; >> int prune; >=20