git: 529a2a0f2765 - main - tcp: For hostcache performance, use atomics instead of counters
Marko Zec
zec at fer.hr
Thu Apr 1 09:35:30 UTC 2021
On Thu, 1 Apr 2021 11:11:39 +0200
Michael Tuexen <michael.tuexen at macmic.franken.de> wrote:
> > On 1. Apr 2021, at 10:20, Marko Zec <zec at fer.hr> wrote:
> >
> > On Thu, 1 Apr 2021 08:03:57 GMT
> > Richard Scheffenegger <rscheff at freebsd.org> wrote:
> >
> >> The branch main has been updated by rscheff:
> >>
> >> URL:
> >> https://cgit.FreeBSD.org/src/commit/?id=529a2a0f2765f6c57c50a5af6be242c03bf714e3
> >>
> >> commit 529a2a0f2765f6c57c50a5af6be242c03bf714e3
> >> Author: Richard Scheffenegger <rscheff at FreeBSD.org>
> >> AuthorDate: 2021-04-01 08:00:32 +0000
> >> Commit: Richard Scheffenegger <rscheff at FreeBSD.org>
> >> CommitDate: 2021-04-01 08:03:30 +0000
> >>
> >> tcp: For hostcache performance, use atomics instead of counters
> >>
> >
> > Is that an April 1st joke, or for real, since it appears to have hit
> > the tree?
> >
> >> As accessing the tcp hostcache happens frequently on some
> >> classes of servers, it was recommended
> >
> > Recommended by whom?
> See https://reviews.freebsd.org/D29510#661680
OK thanks... Perhaps if the commit note stated that it aims to
eliminate counter_u64_fetch() from a hot path that would have spared
the list from the noise from me...
Cheers,
Marko
>
> Best regards
> Michael
> >
> >> to use atomic_add/subtract
> >> rather than (per-CPU distributed) counters, which have to be
> >> summed up at high cost to cache efficiency.
> >
> > Numbers?
> >
> >>
> >> 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(-)
> >>
> >> 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");
> >> -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");
> >>
> >> 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 = 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 = TCP_HOSTCACHE_HASHSIZE;
> >> V_tcp_hostcache.bucket_limit = TCP_HOSTCACHE_BUCKETLIMIT;
> >> V_tcp_hostcache.expire = TCP_HOSTCACHE_EXPIRE;
> >> @@ -268,9 +267,6 @@ tcp_hc_destroy(void)
> >> /* Purge all hc entries. */
> >> tcp_hc_purge_internal(1);
> >>
> >> - /* 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);
> >>
> >> @@ -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 >= V_tcp_hostcache.bucket_limit ||
> >> - counter_u64_fetch(V_tcp_hostcache.cache_count) >=
> >> V_tcp_hostcache.cache_limit) {
> >> + atomic_load_int(&V_tcp_hostcache.cache_count) >=
> >> V_tcp_hostcache.cache_limit) { hc_entry =
> >> 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);
> >>
> >> return hc_entry;
> >> @@ -644,7 +640,8 @@ sysctl_tcp_hc_list(SYSCTL_HANDLER_ARGS)
> >>
> >> /* Optimize Buffer length query by sbin/sysctl */
> >> if (req->oldptr == NULL) {
> >> - len =
> >> (counter_u64_fetch(V_tcp_hostcache.cache_count) + 1) * linesize;
> >> + len =
> >> (atomic_load_int(&V_tcp_hostcache.cache_count)
> >> + 1) *
> >> + linesize;
> >> return (SYSCTL_OUT(req, NULL, len));
> >> }
> >>
> >> @@ -654,7 +651,8 @@ sysctl_tcp_hc_list(SYSCTL_HANDLER_ARGS)
> >> }
> >>
> >> /* 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);
> >>
> >> 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 -=
> >> 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;
> >
>
More information about the dev-commits-src-all
mailing list