Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 25 Feb 2015 10:56:59 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Harrison Grundy <harrison.grundy@astrodoggroup.com>
Cc:        freebsd-arch@freebsd.org
Subject:   Re: locks and kernel randomness...
Message-ID:  <20150225085659.GA74514@kib.kiev.ua>
In-Reply-To: <54ED87E9.8030706@astrodoggroup.com>
References:  <DD06E2EA-68D6-43D7-AA17-FB230750E55A@bsdimp.com> <20150224174053.GG46794@funkthat.com> <54ECBD4B.6000007@freebsd.org> <20150224182507.GI46794@funkthat.com> <54ECEA43.2080008@freebsd.org> <20150224231921.GQ46794@funkthat.com> <CAHM0Q_NhUpr_HJZZcAEoZ_hNvZKcVzUBH-7LALsbkgqjLimA7A@mail.gmail.com> <20150225002301.GS46794@funkthat.com> <54ED80BD.1080603@freebsd.org> <54ED87E9.8030706@astrodoggroup.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Feb 25, 2015 at 12:29:29AM -0800, Harrison Grundy wrote:
> 
> 
> On 02/24/15 23:58, Alfred Perlstein wrote:
> > 
> > On 2/24/15 7:23 PM, John-Mark Gurney wrote:
> >> K. Macy wrote this message on Tue, Feb 24, 2015 at 15:33 -0800:
> >>>> If someone does find a performance issue w/ my patch, I WILL
> >>>> work with them on a solution, but I will not work w/ people
> >>>> who make unfounded claims about the impact of this work...
> >>>> 
> >>> <shrug> ... The concerns may be exaggerated, but they aren't 
> >>> unfounded. Not quite the same thing, but no one wants to spend
> >>> the
> >> Till someone shows me code in the kernel tree where this is even
> >> close to a performance problem, it is unfounded...  I've asked,
> >> and no one has
> >> 
> >>> cycles doing a SHA256 because it's "The Right Thing"(tm) when
> >>> their use case only requires a fletcher2.
> >> Depends upon what you're doing.. I haven't proposed changing
> >> ZFS's default to sha256, so stop w/ the false equivalences...
> >> 
> >>> If it doesn't already exist, it might also be worth looking in
> >>> to a more scalable CSPRNG implementation not requiring locking
> >>> in the common case. For example, each core is seeded separately
> >>> periodically so that has a private pool that is protected by a
> >>> critical section. The private pool would be regularly refreshed
> >>> by cpu-local callout. Thus, a lock would only be acquired if
> >>> the local entropy were depleted.
> >> I'm not discussing this until you read and reply to my original
> >> email, since it's clear that my original email's contents has
> >> been ignored in this thread...
> >> 
> > What is final proposal?  More spinlocks?  That is not a good idea.
> > 
> > Doing a single buildworld is not enough.  Ask netflix or someone
> > with a real load of 1000s of threads/processing to do testing for
> > you if you truly want to touch scheduler.
> 
> sched_ule runs this code once every .5 to 1.5 seconds, depending on
> the value of random, so using a CSPRNG there wouldn't actually be
> noticeable. (We're talking about a few thousand cycles, when the
> existing implementation has to make a remote memory read/write
> numpackages-1/numpackages percent of the time, which costs tens of
> thousands of cycles. Switching to a per-CPU CSPRNG is actually faster
> in those cases.)
The cost of the proposed patch, of course, is not the several
thousands of instructions in the rebalance. The problem with it is the
introduction of the new spinlock, which will be used in many places
after the introduction. The cost of the new and often used spinlock is
the increase of both interrupt latency and interrupt handler jitter and
cpu switch jitter.

So neither buildworld timing, nor network throughput are adequate
to estimate the change.  It is system unresponsivness and loss of
the realtime behaviour up to some degree.

I thought that it was obvious, at least after spinlocks were mentioned,
but apparently it is not, since proposals to measure the patch effect
by benchmarking buildworld or passing the traffic are made.

> 
> That being said, I believe the plan is to remove random() from
> sched_ule entirely. It doesn't need it to perform the balancing, and
> we can just use the LCG from cpu_search, if get_cyclecount isn't viable.

I agree with this, but please see my other response.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150225085659.GA74514>