All of lore.kernel.org
 help / color / mirror / Atom feed
From: Amos Kong <akong@redhat.com>
To: "Michael Büsch" <m@bues.ch>
Cc: herbert@gondor.apana.org.au, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org, mpm@selenic.com,
	amit.shah@redhat.com
Subject: Re: [PATCH v2 3/3] hw_random: increase schedule timeout in rng_dev_read()
Date: Tue, 16 Sep 2014 08:27:40 +0800	[thread overview]
Message-ID: <20140916002740.GA5671@zen> (raw)
In-Reply-To: <20140915181331.4e3f5fed@wiggum>


[-- Attachment #1.1: Type: text/plain, Size: 1930 bytes --]

On Mon, Sep 15, 2014 at 06:13:31PM +0200, Michael Büsch wrote:
> On Tue, 16 Sep 2014 00:02:29 +0800
> Amos Kong <akong@redhat.com> wrote:
> 
> > This patch increases the schedule timeout to 10 jiffies, it's more
> > appropriate, then other takes can easy to hold the mutex lock.
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  drivers/char/hw_random/core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> > index 263a370..b5d1b6f 100644
> > --- a/drivers/char/hw_random/core.c
> > +++ b/drivers/char/hw_random/core.c
> > @@ -195,7 +195,7 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> >  
> >  		mutex_unlock(&rng_mutex);
> >  
> > -		schedule_timeout_interruptible(1);
> > +		schedule_timeout_interruptible(10);
> >  
> >  		if (signal_pending(current)) {
> >  			err = -ERESTARTSYS;
> 
> Does a schedule of 1 ms or 10 ms decrease the throughput?

In my test environment, 1 jiffe always works (100%), as suggested by
Amit 10 jiffes is more appropriate.

After applied current 3 patches, there is a throughput regression.

  1.2 M/s -> 6 K/s

We can only schedule in the end of loop (size == 0), and only for
non-smp guest. So smp guest won't be effected.

|               if (!size && num_online_cpus() == 1)
|                       schedule_timeout_interruptible(timeout);


Set timeout to 1:
  non-smp guest with quick backend (1.2M/s) -> about 49K/s)

Set timeout to 10:
  non-smp guest with quick backend (1.2M/s) -> about 490K/s)

We might need other benchmark to test the performance, but we can
see the bug clearly caused a regression.

As we discussed in other thread, need_resched() should work in this
case, so those patches might be wrong fixing.

> I think we need some benchmarks.
> 
> -- 
> Michael



-- 
			Amos.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

WARNING: multiple messages have this Message-ID (diff)
From: Amos Kong <akong@redhat.com>
To: "Michael Büsch" <m@bues.ch>
Cc: virtualization@lists.linux-foundation.org, kvm@vger.kernel.org,
	herbert@gondor.apana.org.au, mpm@selenic.com,
	rusty@rustcorp.com.au, amit.shah@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 3/3] hw_random: increase schedule timeout in rng_dev_read()
Date: Tue, 16 Sep 2014 08:27:40 +0800	[thread overview]
Message-ID: <20140916002740.GA5671@zen> (raw)
In-Reply-To: <20140915181331.4e3f5fed@wiggum>

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

On Mon, Sep 15, 2014 at 06:13:31PM +0200, Michael Büsch wrote:
> On Tue, 16 Sep 2014 00:02:29 +0800
> Amos Kong <akong@redhat.com> wrote:
> 
> > This patch increases the schedule timeout to 10 jiffies, it's more
> > appropriate, then other takes can easy to hold the mutex lock.
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  drivers/char/hw_random/core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> > index 263a370..b5d1b6f 100644
> > --- a/drivers/char/hw_random/core.c
> > +++ b/drivers/char/hw_random/core.c
> > @@ -195,7 +195,7 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> >  
> >  		mutex_unlock(&rng_mutex);
> >  
> > -		schedule_timeout_interruptible(1);
> > +		schedule_timeout_interruptible(10);
> >  
> >  		if (signal_pending(current)) {
> >  			err = -ERESTARTSYS;
> 
> Does a schedule of 1 ms or 10 ms decrease the throughput?

In my test environment, 1 jiffe always works (100%), as suggested by
Amit 10 jiffes is more appropriate.

After applied current 3 patches, there is a throughput regression.

  1.2 M/s -> 6 K/s

We can only schedule in the end of loop (size == 0), and only for
non-smp guest. So smp guest won't be effected.

|               if (!size && num_online_cpus() == 1)
|                       schedule_timeout_interruptible(timeout);


Set timeout to 1:
  non-smp guest with quick backend (1.2M/s) -> about 49K/s)

Set timeout to 10:
  non-smp guest with quick backend (1.2M/s) -> about 490K/s)

We might need other benchmark to test the performance, but we can
see the bug clearly caused a regression.

As we discussed in other thread, need_resched() should work in this
case, so those patches might be wrong fixing.

> I think we need some benchmarks.
> 
> -- 
> Michael



-- 
			Amos.

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

  reply	other threads:[~2014-09-16  0:27 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-15 16:02 [PATCH v2 0/3] fix stuck in accessing hwrng attributes Amos Kong
2014-09-15 16:02 ` Amos Kong
2014-09-15 16:02 ` [PATCH v2 1/3] virtio-rng cleanup: move some code out of mutex protection Amos Kong
2014-09-15 16:02   ` Amos Kong
2014-09-15 16:13   ` Michael Büsch
2014-09-16  0:30     ` Amos Kong
2014-09-16  0:30       ` Amos Kong
2014-09-15 16:13   ` Michael Büsch
2014-09-15 16:02 ` [PATCH v2 2/3] hw_random: fix stuck in catting hwrng attributes Amos Kong
2014-09-15 16:02   ` Amos Kong
2014-09-18  2:43   ` Rusty Russell
2014-09-18  2:43     ` Rusty Russell
2014-09-18  2:48     ` [PATCH 1/5] hw_random: place mutex around read functions and buffers Rusty Russell
2014-09-18  2:48       ` Rusty Russell
2014-09-18  2:48       ` [PATCH 2/5] hw_random: use reference counts on each struct hwrng Rusty Russell
2014-09-18  2:48       ` Rusty Russell
2014-09-18 12:22         ` Amos Kong
2014-09-18 12:22           ` Amos Kong
2014-09-18  2:48       ` [PATCH 3/5] hw_random: fix unregister race Rusty Russell
2014-09-18  2:48         ` Rusty Russell
2014-10-21 14:15         ` Herbert Xu
2014-10-21 14:15           ` Herbert Xu
2014-11-03 15:24           ` Amos Kong
2014-11-03 15:24             ` Amos Kong
2014-09-18  2:48       ` [PATCH 4/5] hw_random: don't double-check old_rng Rusty Russell
2014-09-18  2:48         ` Rusty Russell
2014-09-18  2:48       ` [PATCH 5/5] hw_random: don't init list element we're about to add to list Rusty Russell
2014-09-18  2:48         ` Rusty Russell
2014-09-18 12:47     ` [PATCH v2 2/3] hw_random: fix stuck in catting hwrng attributes Amos Kong
2014-09-18 12:47       ` Amos Kong
2014-09-15 16:02 ` [PATCH v2 3/3] hw_random: increase schedule timeout in rng_dev_read() Amos Kong
2014-09-15 16:02 ` Amos Kong
2014-09-15 16:13   ` Michael Büsch
2014-09-15 16:13   ` Michael Büsch
2014-09-16  0:27     ` Amos Kong [this message]
2014-09-16  0:27       ` Amos Kong
2014-09-16 15:01       ` Michael Büsch
2014-09-16 15:01         ` Michael Büsch
2014-09-17  9:30 ` [PATCH v2 0/3] fix stuck in accessing hwrng attributes Herbert Xu
2014-09-17  9:30   ` Herbert Xu

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=20140916002740.GA5671@zen \
    --to=akong@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m@bues.ch \
    --cc=mpm@selenic.com \
    --cc=virtualization@lists.linux-foundation.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.