All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@digeo.com>
To: lkml <linux-kernel@vger.kernel.org>
Subject: the random driver
Date: Tue, 19 Nov 2002 23:46:53 -0800	[thread overview]
Message-ID: <3DDB3DED.A4C9DC56@digeo.com> (raw)


I'm seeing a context switch rate of 150/sec just writing a
big file to disk at 20 megabytes/sec.

It is coming out of add_disk_randomness()'s invokation of
batch_entropy_store().

That function is setting up deferred punt-to-process-context
for every disk request, and has the potential to cause 1000
context switches per second.  This is clearly excessive.

There is a 256 slot buffer in the random driver for this,
and we are not using it at all effectively.  I do intend
to submit the below patch which will cause one context switch
per 128 requests.

But this is a minimal fix.  The batch_entropy_pool handling
in there needs work.

a) It's racy.  The head and tail pointers have no SMP protection
   and a race will cause it to dump 128 already-processed items
   back into the entropy pool.

b) It's weird.  What's up with this?

        batch_entropy_pool[2*batch_head] = a;
        batch_entropy_pool[(2*batch_head) + 1] = b;

   It should be an array of 2-element structures.

c) The ring-buffer handling is awkward.  It shouldn't be masking
   the head and tail pointers to always remain within bounds.

   A better technique is to allow these indices to wrap at
   0xffffffff and only mask their values when you actually use
   them as a subscript.  This allows you to distinguish the
   completely-full case from the completely-empty one.  See
   LOG_BUF* in kernel/printk.c.

d) It's punting work up to process context which could be performed
   right there in interrupt context.

My suggestion, if anyone cares, is to convert the entropy pool
into smaller per-cpu buffers, protected by local_irq_save() only.
This way the global lock (which isn't there yet) only needs to
be taken when a CPU is actually dumping its buffer.



--- 25/drivers/char/random.c~reduce-random-context-switch-rate	Tue Nov 19 23:17:12 2002
+++ 25-akpm/drivers/char/random.c	Tue Nov 19 23:18:11 2002
@@ -663,7 +663,7 @@ void batch_entropy_store(u32 a, u32 b, i
 	batch_entropy_credit[batch_head] = num;
 
 	new = (batch_head+1) & (batch_max-1);
-	if (new != batch_tail) {
+	if ((new - batch_tail) >= batch_max / 2) {
 		/*
 		 * Schedule it for the next timer tick:
 		 */

_

             reply	other threads:[~2002-11-20  7:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-11-20  7:46 Andrew Morton [this message]
2002-11-20  8:13 ` the random driver Aaron Lehmann
2002-11-20 20:44   ` Oliver Xymoron
2002-11-20 12:04 ` Ingo Oeser
2002-11-20 16:27 ` Theodore Ts'o
2002-11-20 19:35   ` Andrew Morton
2002-11-20 20:42 ` Oliver Xymoron

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=3DDB3DED.A4C9DC56@digeo.com \
    --to=akpm@digeo.com \
    --cc=linux-kernel@vger.kernel.org \
    /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.