All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"David S.Miller" <davem@davemloft.net>,
	Ingo Molnar <mingo@kernel.org>,
	Chandramouli Narayanan <mouli@linux.intel.com>,
	Vinodh Gopal <vinodh.gopal@intel.com>,
	James Guilford <james.guilford@intel.com>,
	Wajdi Feghali <wajdi.k.feghali@intel.com>,
	Jussi Kivilinna <jussi.kivilinna@iki.fi>,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 6/7] sched: add function nr_running_cpu to expose number of tasks running on cpu
Date: Tue, 15 Jul 2014 11:50:45 +0200	[thread overview]
Message-ID: <20140715095045.GV9918@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1405367450.2970.750.camel@schen9-DESK>

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

On Mon, Jul 14, 2014 at 12:50:50PM -0700, Tim Chen wrote:

> There is a generic multi-buffer infrastructure portion that manages
> pulling and queuing jobs on the crypto workqueue, and it is separated out
> in patch 1 of the patchset.

There's one very weird multi-line comment in that patch.

> The other portions are algorithm specific that defines
> algorithm specific data structure and  does the crypto computation 
> for a particular algorithm, mostly in
> assemblies and C glue code.  The infrastructure code is 
> meant to be reused for other similar 
> multi-buffer algorithms.

The flushing part that uses the sched thing is sha1 specific, even
though it strikes me as not being so. Flushing buffers on idle seems
like a 'generic' thing.

> We use nr_running_cpu to check whether there are other tasks running on
> the *current* cpu, (not for another cpu),

And yet, the function allows you do to exactly that..

> to decide if we should flush
> and compute crypto jobs accumulated.  If there's nobody else running,
> we can take advantage of available cpu cycles on the cpu we are running 
> on to do computation on the existing jobs in a SIMD mannner. 
> Waiting a bit longer may accumulate more jobs to process in parallel
> in a single SIMD instruction, but will have more delay.  

So you already have an idle notifier (which is x86 only, we should fix
that I suppose), and you then double check there really isn't anything
else running.

How much, if anything, does that second check buy you? There's just not
a single word on that.

Also, there is not a word on the latency vs throughput tradeoff you
make. I can imagine that for very short idle durations you loose, not
win with this thing.

So for now I still see no reason for doing this.

Also, I wonder about SMT, the point of this is to make best use of the
SIMD pipelines, does it still make sense to use siblings at the same
time even though you're running hand crafted ASM to stuff the pipelines
to the brim? Should this thing be SMT aware and not gather queues for
both siblings?

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2014-07-15  9:51 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1405074379.git.tim.c.chen@linux.intel.com>
2014-07-11 20:32 ` [PATCH v4 0/7] crypto: SHA1 multibuffer implementation Tim Chen
2014-07-11 20:32 ` [PATCH v4 1/7] crypto: SHA1 multibuffer crypto hash infrastructure Tim Chen
2014-07-11 20:32 ` [PATCH v4 2/7] crypto: SHA1 multibuffer algorithm data structures Tim Chen
2014-07-11 20:32 ` [PATCH v4 3/7] crypto: SHA1 multibuffer submit and flush routines for AVX2 Tim Chen
2014-07-11 20:32 ` [PATCH v4 4/7] crypto: SHA1 multibuffer crypto computation (x8 AVX2) Tim Chen
2014-07-11 20:33 ` [PATCH v4 5/7] crypto: SHA1 multibuffer scheduler Tim Chen
2014-07-11 20:33 ` [PATCH v4 6/7] sched: add function nr_running_cpu to expose number of tasks running on cpu Tim Chen
2014-07-12  9:25   ` Kirill Tkhai
2014-07-14 17:51     ` Tim Chen
2014-07-12 14:21   ` Tadeusz Struk
2014-07-14 23:51     ` Tim Chen
2014-07-14 10:16   ` Peter Zijlstra
2014-07-14 16:10     ` Tim Chen
2014-07-14 16:14       ` Peter Zijlstra
2014-07-14 17:05         ` Tim Chen
2014-07-14 18:17           ` Peter Zijlstra
2014-07-14 19:08             ` Tim Chen
2014-07-14 19:15               ` Peter Zijlstra
2014-07-14 19:50                 ` Tim Chen
2014-07-15  9:50                   ` Peter Zijlstra [this message]
2014-07-15 12:07                     ` Peter Zijlstra
2014-07-15 12:59                       ` Thomas Gleixner
2014-07-15 14:45                         ` Mike Galbraith
2014-07-15 14:53                           ` Peter Zijlstra
2014-07-15 18:06                             ` Mike Galbraith
2014-07-15 19:03                               ` Peter Zijlstra
2014-07-15 19:24                                 ` Mike Galbraith
2014-07-15 18:41                         ` Tim Chen
2014-07-15 20:46                           ` Thomas Gleixner
2014-07-15 18:40                       ` Tim Chen
2014-07-15 18:40                     ` Tim Chen
2014-07-15 13:36                 ` Peter Zijlstra
2014-07-15 15:21                   ` Tejun Heo
2014-07-15 16:37                     ` Peter Zijlstra
2014-07-11 20:33 ` [PATCH v4 7/7] crypto: SHA1 multibuffer - flush the jobs early if cpu becomes idle Tim Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140715095045.GV9918@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=hpa@zytor.com \
    --cc=james.guilford@intel.com \
    --cc=jussi.kivilinna@iki.fi \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mouli@linux.intel.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=vinodh.gopal@intel.com \
    --cc=wajdi.k.feghali@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.