Date: Fri, 11 May 2018 06:48:33 -0700 (PDT) From: "Rodney W. Grimes" <freebsd@pdx.rh.CN85.dnsmgr.net> To: Mateusz Guzik <mjg@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r333484 - head/sys/vm Message-ID: <201805111348.w4BDmXbe075549@pdx.rh.CN85.dnsmgr.net> In-Reply-To: <201805110704.w4B74wA2037309@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[ Charset UTF-8 unsupported, converting... ] > Author: mjg > Date: Fri May 11 07:04:57 2018 > New Revision: 333484 > URL: https://svnweb.freebsd.org/changeset/base/333484 > > Log: > uma: increase alignment to 128 bytes on amd64 > > Current UMA internals are not suited for efficient operation in > multi-socket environments. In particular there is very common use of > MAXCPU arrays and other fields which are not always properly aligned and > are not local for target threads (apart from the first node of course). > Turns out the existing UMA_ALIGN macro can be used to mostly work around > the problem until the code get fixed. The current setting of 64 bytes > runs into trouble when adjacent cache line prefetcher gets to work. > > An example 128-way benchmark doing a lot of malloc/frees has the following > instruction samples: What are the effects of this on 4, 8, and 16-way systems? Is this more optimization for big iron that pessimizes little iron? I find it odd that changing something from a cache line size dependent to a hardcoded value is an optimization for all. I understand that this is coming about because the value of CACHE_LINE_SIZE was changed, but is this really the correct fix? Should work be started to stop the assumption that we can compile a kernel for all CACHE line sizes and start to adjust things at load time? There are lots of places that CACHE size and topology effect what are optimal values, and this stuffing of constants is just not a long term workable solution. If ANYTHING this constant 128 should become some type of #define so that it can be tuned at compile time, or wrapped in #ifndef, see below. > before: > kernel`lf_advlockasync+0x43b 32940 > kernel`malloc+0xe5 42380 > kernel`bzero+0x19 47798 > kernel`spinlock_exit+0x26 60423 > kernel`0xffffffff80 78238 > 0x0 136947 > kernel`uma_zfree_arg+0x46 159594 > kernel`uma_zalloc_arg+0x672 180556 > kernel`uma_zfree_arg+0x2a 459923 > kernel`uma_zalloc_arg+0x5ec 489910 > > after: > kernel`bzero+0xd 46115 > kernel`lf_advlockasync+0x25f 46134 > kernel`lf_advlockasync+0x38a 49078 > kernel`fget_unlocked+0xd1 49942 > kernel`lf_advlockasync+0x43b 55392 > kernel`copyin+0x4a 56963 > kernel`bzero+0x19 81983 > kernel`spinlock_exit+0x26 91889 > kernel`0xffffffff80 136357 > 0x0 239424 > > See the review for more details. > > Reviewed by: kib > Differential Revision: https://reviews.freebsd.org/D15346 > > Modified: > head/sys/vm/uma_int.h > > Modified: head/sys/vm/uma_int.h > ============================================================================== > --- head/sys/vm/uma_int.h Fri May 11 06:59:54 2018 (r333483) > +++ head/sys/vm/uma_int.h Fri May 11 07:04:57 2018 (r333484) > @@ -177,7 +177,7 @@ struct uma_hash { > * align field or structure to cache line > */ #ifndef UMA_ALIGN > #if defined(__amd64__) > -#define UMA_ALIGN __aligned(CACHE_LINE_SIZE) > +#define UMA_ALIGN __aligned(128) > #else > #define UMA_ALIGN > #endif #endif > > -- Rod Grimes rgrimes@freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201805111348.w4BDmXbe075549>