-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 11/02/14 12:53, Ian Lepore wrote:
> On Sun, 2014-11-02 at 12:27 -0800, Xin Li wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
>> 
>> Hi, Mark,
>> 
>> I'd like to propose the attached patch for review.  It replaces 
>> tsleep's with sx_sleep's, then checks the return value and quit
>> the loop.
>> 
>> Cheers, - --
> 
> It still doesn't handle the partial read/write case Kostik
> mentioned, but there are plenty of other drivers that don't get
> that right.  Given that the ra_read/ra_write functions can't return
> error, it would only be errors from uiomove() in play.  I guess it
> would be something like this:
> 
> nbytes = uio->uio_resid; while (uio->uio_resid && !error) { c =
> MIN(uio->uio_resid, PAGE_SIZE); 
> (random_adaptor->ra_read)(random_buf, c); error =
> uiomove(random_buf, c, uio); } if (uio->uio_resid != nbytes) error
> = 0; /* Return partial read, not error. */
> 
> Also, there's now a mix of if (error == 0) and if (!error) near
> each other (I tend to prefer using ! only on boolean_t, but I even
> more prefer local consistancy. :)

Thanks for pointing this out.

I've added some checks at:

        http://pastebin.com/ZGsqpUHR

Also attached.

Cheers,
- -- 
Xin LI <delp...@delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0

iQIcBAEBCgAGBQJUVqR+AAoJEJW2GBstM+nsLl8QAIFQQZCGwwkeTlVkG/i9YzAl
pK4uox2dXouN1Zfwi9Q3ymB0DsUbnXt9IEP9JCD4Z8ZeFKUIC2mNI/V+t3etV5Kr
kVdvayizwbJ0jLBYu6rUIoeZeqWFnLB25PitJFCPOAqJ+H1h/hV56Zx9SvkAWEIe
spGpvq+eeHRYl/AzVMQFHjax0V7bo3yd+3klDlU3DT77WFvSRngNKUxHgVPMsoBY
xm4YRPy6MLAHXlEoHtoERc5Qyhj00Km4ZRHtXn3tFh6LSAdtXmbBiVXlHWZ0eVEn
5vH0d3MKRCnKtQg2ydEA5SWoCTMHIVCfoB/yQ4B0N0FupgdEUXMlag0iBV8DZcmb
nv1sd68V3GgDjiiAwevhbSLQ0cHr5FABtTdXwExClR2sOh3rN1PONsfbzfs3FgkO
TWts6znUcblmpCFyQyz49/r+SQ35S1YXCKIZB5Z43yysPgR20uuSKbD0XKAne28u
acntrXXgrfERSue1T5qz2xwH+dBWGuJtGDq1wSeP1kw8aWoX+ApAbUColCY2pEhm
OawGlJ6qnr4VgEIl4vQ/S9gaxMX33K/KCHJERaCA/8h1WQQklmRTky4URZ6bAaHJ
NMEd6GYD25GmfIY7kHB/tuPfjiWDwGnNh8mvSSAZBJM81gW+/9xqKwKn3OZ3kMFY
eM+SpnlDm4skN3geKGse
=yQhY
-----END PGP SIGNATURE-----
Index: sys/dev/random/random_adaptors.c
===================================================================
--- sys/dev/random/random_adaptors.c    (revision 273982)
+++ sys/dev/random/random_adaptors.c    (working copy)
@@ -171,9 +171,8 @@ random_adaptor_register(const char *name, struct r
        sx_xlock(&random_adaptors_lock);
        LIST_INSERT_HEAD(&random_adaptors_list, rra, rra_entries);
        random_adaptor_choose();
+       KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
        sx_xunlock(&random_adaptors_lock);
-
-       KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
 }
 
 void
@@ -182,9 +181,9 @@ random_adaptor_deregister(const char *name)
        struct random_adaptors *rra;
 
        KASSERT(name != NULL, ("invalid input to %s", __func__));
-       KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
 
        sx_xlock(&random_adaptors_lock);
+       KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
        LIST_FOREACH(rra, &random_adaptors_list, rra_entries)
                if (strcmp(rra->rra_name, name) == 0) {
                        LIST_REMOVE(rra, rra_entries);
@@ -208,16 +207,18 @@ random_adaptor_read(struct cdev *dev __unused, str
        printf("random: %s %ld\n", __func__, uio->uio_resid);
 #endif
 
-       KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
+       random_buf = malloc(PAGE_SIZE, M_ENTROPY, M_WAITOK);
 
        sx_slock(&random_adaptors_lock);
 
+       KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
+
        /* Let the entropy source do any pre-read setup. */
        (random_adaptor->ra_read)(NULL, 0);
 
        /* (Un)Blocking logic */
        error = 0;
-       while (!random_adaptor->ra_seeded()) {
+       while (!random_adaptor->ra_seeded() && error == 0) {
                if (flags & O_NONBLOCK) {
                        error = EWOULDBLOCK;
                        break;
@@ -224,7 +225,10 @@ random_adaptor_read(struct cdev *dev __unused, str
                }
 
                /* Sleep instead of going into a spin-frenzy */
-               tsleep(&random_adaptor, PUSER | PCATCH, "block", hz/10);
+               error = sx_sleep(&random_adaptor, &random_adaptors_lock,
+                   PUSER | PCATCH, "block", hz/10);
+               KASSERT(random_adaptor != NULL, ("No active random adaptor in 
%s",
+                   __func__));
 
                /* keep tapping away at the pre-read until we seed/unblock. */
                (random_adaptor->ra_read)(NULL, 0);
@@ -241,12 +245,10 @@ random_adaptor_read(struct cdev *dev __unused, str
 
        mtx_unlock(&random_read_rate_mtx);
 
-       if (!error) {
+       if (error == 0) {
+               nbytes = uio->uio_resid;
 
                /* The actual read */
-
-               random_buf = malloc(PAGE_SIZE, M_ENTROPY, M_WAITOK);
-
                while (uio->uio_resid && !error) {
                        c = MIN(uio->uio_resid, PAGE_SIZE);
                        (random_adaptor->ra_read)(random_buf, c);
@@ -256,11 +258,15 @@ random_adaptor_read(struct cdev *dev __unused, str
                /* Let the entropy source do any post-read cleanup. */
                (random_adaptor->ra_read)(NULL, 1);
 
-               free(random_buf, M_ENTROPY);
+               if (nbytes != uio->uio_resid && (error == ERESTART ||
+                   error == EINTR) )
+                       error = 0;      /* Return partial read, not error. */
+
        }
-
        sx_sunlock(&random_adaptors_lock);
 
+       free(random_buf, M_ENTROPY);
+
        return (error);
 }
 
@@ -269,6 +275,8 @@ random_adaptor_read_rate(void)
 {
        int ret;
 
+       sx_assert(&random_adaptors_lock, SA_LOCKED);
+
        KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
 
        mtx_lock(&random_read_rate_mtx);
@@ -287,18 +295,20 @@ random_adaptor_write(struct cdev *dev __unused, st
 {
        int c, error = 0;
        void *random_buf;
+       ssize_t nbytes;
 
 #ifdef RANDOM_DEBUG
        printf("random: %s %zd\n", __func__, uio->uio_resid);
 #endif
 
-       KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
+       random_buf = malloc(PAGE_SIZE, M_ENTROPY, M_WAITOK);
 
        sx_slock(&random_adaptors_lock);
 
-       random_buf = malloc(PAGE_SIZE, M_ENTROPY, M_WAITOK);
+       KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
 
-       while (uio->uio_resid > 0) {
+       nbytes = uio->uio_resid;
+       while (uio->uio_resid > 0 && error == 0) {
                c = MIN(uio->uio_resid, PAGE_SIZE);
                error = uiomove(random_buf, c, uio);
                if (error)
@@ -306,13 +316,20 @@ random_adaptor_write(struct cdev *dev __unused, st
                (random_adaptor->ra_write)(random_buf, c);
 
                /* Introduce an annoying delay to stop swamping */
-               tsleep(&random_adaptor, PUSER | PCATCH, "block", hz/10);
+               error = sx_sleep(&random_adaptor, &random_adaptors_lock,
+                   PUSER | PCATCH, "block", hz/10);
+               KASSERT(random_adaptor != NULL, ("No active random adaptor in 
%s",
+                   __func__));
        }
 
+       sx_sunlock(&random_adaptors_lock);
+
+       if (nbytes != uio->uio_resid && (error == ERESTART ||
+           error == EINTR) )
+               error = 0;      /* Partial write, not error. */
+
        free(random_buf, M_ENTROPY);
 
-       sx_sunlock(&random_adaptors_lock);
-
        return (error);
 }
 
@@ -325,10 +342,10 @@ random_adaptor_poll(struct cdev *dev __unused, int
        printf("random: %s\n", __func__);
 #endif
 
+       sx_slock(&random_adaptors_lock);
+
        KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
 
-       sx_slock(&random_adaptors_lock);
-
        if (events & (POLLIN | POLLRDNORM)) {
                if (random_adaptor->ra_seeded())
                        events &= (POLLIN | POLLRDNORM);
@@ -382,9 +399,9 @@ random_sysctl_active_adaptor_handler(SYSCTL_HANDLE
        struct sbuf sbuf;
        int error;
 
+       sx_slock(&random_adaptors_lock);
        KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
 
-       sx_slock(&random_adaptors_lock);
        sbuf_new_for_sysctl(&sbuf, NULL, 16, req);
        LIST_FOREACH(rra, &random_adaptors_list, rra_entries)
                if (rra->rra_ra == random_adaptor) {
@@ -454,9 +471,9 @@ static void
 random_adaptors_seed(void *unused __unused)
 {
  
+       sx_slock(&random_adaptors_lock);
        KASSERT(random_adaptor != NULL, ("No active random adaptor in %s", 
__func__));
 
-       sx_slock(&random_adaptors_lock);
        random_adaptor->ra_reseed();
        sx_sunlock(&random_adaptors_lock);
 

Attachment: random-tsleep.diff.sig
Description: Binary data

_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to