All of lore.kernel.org
 help / color / mirror / Atom feed
From: Amos Kong <akong@redhat.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: herbert@gondor.apana.org.au, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org, m@bues.ch,
	mpm@selenic.com, amit.shah@redhat.com
Subject: Re: [PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.
Date: Mon, 20 Oct 2014 05:58:30 +0200	[thread overview]
Message-ID: <20141020035830.GA3982@air.redhat.com> (raw)
In-Reply-To: <871tq3a9b8.fsf@rustcorp.com.au>

On Mon, Oct 20, 2014 at 10:42:11AM +1030, Rusty Russell wrote:
> Amos Kong <akong@redhat.com> writes:
> > We got a warning in boot stage when above set_current_rng() is executed,
> > it can be fixed by init rng->ref in hwrng_init().
> >
> >
> > @@ -166,6 +169,8 @@ static inline int hwrng_init(struct hwrng *rng)
> >         if (current_quality > 0 && !hwrng_fill)
> >                 start_khwrngd();
> >  
> > +       kref_init(&rng->ref);
> > +
> >         return 0;
> >  }
> 
> OK, I folded this fix on.

Thanks.

Reviewed-by: Amos Kong <akong@redhat.com>
 
> Thanks,
> Rusty.
> 
> hw_random: use reference counts on each struct hwrng.
> 
> current_rng holds one reference, and we bump it every time we want
> to do a read from it.
> 
> This means we only hold the rng_mutex to grab or drop a reference,
> so accessing /sys/devices/virtual/misc/hw_random/rng_current doesn't
> block on read of /dev/hwrng.
> 
> Using a kref is overkill (we're always under the rng_mutex), but
> a standard pattern.
> 
> This also solves the problem that the hwrng_fillfn thread was
> accessing current_rng without a lock, which could change (eg. to NULL)
> underneath it.
> 
> v3: initialize kref (thanks Amos Kong)
> v2: fix missing put_rng() on exit path (thanks Amos Kong)
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index a0905c818e13..0ecac38da954 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
....

WARNING: multiple messages have this Message-ID (diff)
From: Amos Kong <akong@redhat.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: virtualization@lists.linux-foundation.org,
	herbert@gondor.apana.org.au, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, m@bues.ch, mpm@selenic.com,
	amit.shah@redhat.com
Subject: Re: [PATCH v2 3/6] hw_random: use reference counts on each struct hwrng.
Date: Mon, 20 Oct 2014 05:58:30 +0200	[thread overview]
Message-ID: <20141020035830.GA3982@air.redhat.com> (raw)
In-Reply-To: <871tq3a9b8.fsf@rustcorp.com.au>

On Mon, Oct 20, 2014 at 10:42:11AM +1030, Rusty Russell wrote:
> Amos Kong <akong@redhat.com> writes:
> > We got a warning in boot stage when above set_current_rng() is executed,
> > it can be fixed by init rng->ref in hwrng_init().
> >
> >
> > @@ -166,6 +169,8 @@ static inline int hwrng_init(struct hwrng *rng)
> >         if (current_quality > 0 && !hwrng_fill)
> >                 start_khwrngd();
> >  
> > +       kref_init(&rng->ref);
> > +
> >         return 0;
> >  }
> 
> OK, I folded this fix on.

Thanks.

Reviewed-by: Amos Kong <akong@redhat.com>
 
> Thanks,
> Rusty.
> 
> hw_random: use reference counts on each struct hwrng.
> 
> current_rng holds one reference, and we bump it every time we want
> to do a read from it.
> 
> This means we only hold the rng_mutex to grab or drop a reference,
> so accessing /sys/devices/virtual/misc/hw_random/rng_current doesn't
> block on read of /dev/hwrng.
> 
> Using a kref is overkill (we're always under the rng_mutex), but
> a standard pattern.
> 
> This also solves the problem that the hwrng_fillfn thread was
> accessing current_rng without a lock, which could change (eg. to NULL)
> underneath it.
> 
> v3: initialize kref (thanks Amos Kong)
> v2: fix missing put_rng() on exit path (thanks Amos Kong)
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index a0905c818e13..0ecac38da954 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
....

  reply	other threads:[~2014-10-20  3:58 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-18 12:37 [PATCH v2 0/6] fix hw_random stuck Amos Kong
2014-09-18 12:37 ` Amos Kong
2014-09-18 12:37 ` [PATCH v2 1/6] hw_random: place mutex around read functions and buffers Amos Kong
2014-09-18 12:37   ` Amos Kong
2014-09-18 12:37 ` [PATCH v2 2/6] hw_random: move some code out mutex_lock for avoiding underlying deadlock Amos Kong
2014-09-18 12:37   ` Amos Kong
2014-09-18 12:37 ` [PATCH v2 3/6] hw_random: use reference counts on each struct hwrng Amos Kong
2014-09-18 12:37 ` Amos Kong
2014-09-18 17:08   ` Amos Kong
2014-10-20  0:12     ` Rusty Russell
2014-10-20  0:12     ` Rusty Russell
2014-10-20  3:58       ` Amos Kong [this message]
2014-10-20  3:58         ` Amos Kong
2014-10-20  0:08   ` Rusty Russell
2014-10-20  0:08     ` Rusty Russell
2014-09-18 12:37 ` [PATCH v2 4/6] hw_random: fix unregister race Amos Kong
2014-09-18 12:37   ` Amos Kong
2014-10-21 14:17   ` Herbert Xu
2014-10-21 14:17     ` Herbert Xu
2014-10-30 23:58     ` Rusty Russell
2014-10-30 23:58       ` Rusty Russell
2014-10-31  7:23       ` Herbert Xu
2014-10-31  7:23         ` Herbert Xu
2014-11-02 15:06         ` Amos Kong
2014-11-02 15:06           ` Amos Kong
2014-11-02 15:08           ` Herbert Xu
2014-11-02 15:08             ` Herbert Xu
2014-11-02 15:14             ` Amos Kong
2014-11-02 15:14               ` Amos Kong
2014-09-18 12:37 ` [PATCH v2 5/6] hw_random: don't double-check old_rng Amos Kong
2014-09-18 12:37   ` Amos Kong
2014-09-18 12:37 ` [PATCH v2 6/6] hw_random: don't init list element we're about to add to list Amos Kong
2014-09-18 12:37 ` Amos Kong

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=20141020035830.GA3982@air.redhat.com \
    --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=rusty@rustcorp.com.au \
    --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.