linux-um archives
 help / color / mirror / Atom feed
From: Richard Weinberger <richard@nod.at>
To: Dark <dark@volatile.bz>
Cc: Richard Weinberger <richard.weinberger@gmail.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	anton ivanov <anton.ivanov@cambridgegreys.com>,
	linux-um <linux-um@lists.infradead.org>
Subject: Re: [PATCH] um: Rewrite host RNG driver.
Date: Thu, 29 Aug 2019 17:30:59 +0200 (CEST)	[thread overview]
Message-ID: <1851013915.76434.1567092659763.JavaMail.zimbra@nod.at> (raw)
In-Reply-To: <20190829103628.61953f50@thedarkness.local>

----- Ursprüngliche Mail -----
> Von: "Dark" <dark@volatile.bz>
> An: "Richard Weinberger" <richard.weinberger@gmail.com>, "linux-kernel" <linux-kernel@vger.kernel.org>
> CC: "richard" <richard@nod.at>, "anton ivanov" <anton.ivanov@cambridgegreys.com>, "linux-um"
> <linux-um@lists.infradead.org>
> Gesendet: Donnerstag, 29. August 2019 16:36:28
> Betreff: Re: [PATCH] um: Rewrite host RNG driver.

> On Thu, 29 Aug 2019 15:26:24 +0200, Richard Weinberger
> <richard.weinberger@gmail.com> wrote:
>> So, you removed -EAGAIN handling, made everything synchronous,
>> and changed the interface.t
>> I'm not sure if this really a much better option.
> 
> I should have been more clear here that I'm using the interfaces
> provided by `drivers/char/hw_random/core.c` for consistency with the
> other hardware RNG drivers and to avoid reimplementing stuff that's
> already there.

I got this, and this is a good thing!
 
> It might be a bit hard to see in the diff, but I pass the file
> descriptor to `os_set_fd_async()` to prevent it from blocking.

Well, it does not block but passing -EAGAIN directly back is not nice.
Or does the hw_random framework handle this?

> For the -EAGAIN handling, I'm passing it onto the caller. Since you
> mentioned it, It would be better to handle it in the driver itself
> so I'll update the patch to address that.
> 
>> Rewriting the driver in a modern manner is a good thing, but throwing the
>> old one way with a little hand weaving just because of a unspecified issue
>> is a little harsh.
>> Can you at lest provide more infos what problem you're facing with the
>> old driver?
> 
> Most of it boiled down to it silently breaking if /dev/random on the
> host were to block for any reason, and there was the userspace tool
> requirement to properly make use of it. With that said, the interface
> was also inconsistent with the other hardware RNG drivers which would
> require a rewrite to address anyway.

Maybe our -EAGAIN handling is buggy.
That said I'm all for changing the driver to use the right framework
but please make sure that we don't drop useful stuff like -EAGAIN handling.

Thanks,
//richard


  reply	other threads:[~2019-08-29 15:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-29  1:44 [PATCH] um: Rewrite host RNG driver Alexander Neville
2019-08-29 13:26 ` Richard Weinberger
2019-08-29 14:36   ` Dark
2019-08-29 15:30     ` Richard Weinberger [this message]
2019-08-29 17:10       ` Dark
2019-08-29 17:27         ` Johannes Berg
2019-08-29 13:37 ` Johannes Berg

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=1851013915.76434.1567092659763.JavaMail.zimbra@nod.at \
    --to=richard@nod.at \
    --cc=anton.ivanov@cambridgegreys.com \
    --cc=dark@volatile.bz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=richard.weinberger@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox