From patchwork Wed Sep 27 06:35:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pankaj Gupta X-Patchwork-Id: 9973209 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1ED0B60375 for ; Wed, 27 Sep 2017 06:35:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 13A29289F8 for ; Wed, 27 Sep 2017 06:35:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 085C2290B4; Wed, 27 Sep 2017 06:35:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B1E2E289F8 for ; Wed, 27 Sep 2017 06:35:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752070AbdI0Gfc (ORCPT ); Wed, 27 Sep 2017 02:35:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54712 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751134AbdI0Gfa (ORCPT ); Wed, 27 Sep 2017 02:35:30 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 22F50A850; Wed, 27 Sep 2017 06:35:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 22F50A850 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=pagupta@redhat.com Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9ABD77BAEF; Wed, 27 Sep 2017 06:35:26 +0000 (UTC) Received: from zmail21.collab.prod.int.phx2.redhat.com (zmail21.collab.prod.int.phx2.redhat.com [10.5.83.24]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id CD90D4EE4F; Wed, 27 Sep 2017 06:35:25 +0000 (UTC) Date: Wed, 27 Sep 2017 02:35:25 -0400 (EDT) From: Pankaj Gupta To: Dmitry Torokhov Cc: Amos Kong , linux-crypto@vger.kernel.org, virtualization@lists.linux-foundation.org, Herbert Xu , Rusty Russell , kvm@vger.kernel.org, Michael Buesch , Matt Mackall , amit shah , lkml Message-ID: <1519544875.14829746.1506494125339.JavaMail.zimbra@redhat.com> In-Reply-To: <20170926165241.GB14833@dtor-ws> References: <1418028640-4891-1-git-send-email-akong@redhat.com> <1418028640-4891-2-git-send-email-akong@redhat.com> <369186187.14365871.1506407817157.JavaMail.zimbra@redhat.com> <20170926165241.GB14833@dtor-ws> Subject: Re: [PATCH v5 REPOST 1/6] hw_random: place mutex around read functions and buffers. MIME-Version: 1.0 X-Originating-IP: [10.65.193.190, 10.4.195.6] Thread-Topic: hw_random: place mutex around read functions and buffers. Thread-Index: 08YxA36eHr3Vupszofa8BJanvFDBCA== X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 27 Sep 2017 06:35:30 +0000 (UTC) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > > On Tue, Sep 26, 2017 at 02:36:57AM -0400, Pankaj Gupta wrote: > > > > > > > > A bit late to a party, but: > > > > > > On Mon, Dec 8, 2014 at 12:50 AM, Amos Kong wrote: > > > > From: Rusty Russell > > > > > > > > There's currently a big lock around everything, and it means that we > > > > can't query sysfs (eg /sys/devices/virtual/misc/hw_random/rng_current) > > > > while the rng is reading. This is a real problem when the rng is slow, > > > > or blocked (eg. virtio_rng with qemu's default /dev/random backend) > > > > > > > > This doesn't help (it leaves the current lock untouched), just adds a > > > > lock to protect the read function and the static buffers, in > > > > preparation > > > > for transition. > > > > > > > > Signed-off-by: Rusty Russell > > > > --- > > > ... > > > > > > > > @@ -160,13 +166,14 @@ static ssize_t rng_dev_read(struct file *filp, > > > > char > > > > __user *buf, > > > > goto out_unlock; > > > > } > > > > > > > > + mutex_lock(&reading_mutex); > > > > > > I think this breaks O_NONBLOCK: we have hwrng core thread that is > > > constantly pumps underlying rng for data; the thread takes the mutex > > > and calls rng_get_data() that blocks until RNG responds. This means > > > that even user specified O_NONBLOCK here we'll be waiting until > > > [hwrng] thread releases reading_mutex before we can continue. > > > > I think for 'virtio_rng' for 'O_NON_BLOCK' 'rng_get_data' returns > > without waiting for data which can let mutex to be used by other > > threads waiting if any? > > > > rng_dev_read > > rng_get_data > > virtio_read > > As I said in the paragraph above the code that potentially holds the > mutex for long time is the thread in hwrng core: hwrng_fillfn(). As it > calls rng_get_data() with "wait" argument == 1 it may block while > holding reading_mutex, which, in turn, will block rng_dev_read(), even > if it was called with O_NONBLOCK. yes, 'hwrng_fillfn' does not consider O_NONBLOCK and can result in mutex wait for other tasks. What if we pass zero for wait to 'hwrng_fill' to return early if there is no data? Thanks, Pankaj > > Thanks. > > -- > Dmitry > --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -403,7 +403,7 @@ static int hwrng_fillfn(void *unused) break; mutex_lock(&reading_mutex); rc = rng_get_data(rng, rng_fillbuf, - rng_buffer_size(), 1); + rng_buffer_size(), 0); mutex_unlock(&reading_mutex); put_rng(rng); if (rc <= 0) {