Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 3 Nov 2014 12:13:52 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        d@delphij.net
Cc:        "src-committers@freebsd.org" <src-committers@freebsd.org>, Ian Lepore <ian@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Mark R V Murray <mark@grondar.org>, Konstantin Belousov <kostikbel@gmail.com>
Subject:   Re: svn commit: r273958 - head/sys/dev/random
Message-ID:  <20141103115402.H3149@besplex.bde.org>
In-Reply-To: <5456A69C.6040902@delphij.net>
References:  <201411020201.sA221unt091493@svn.freebsd.org>  <720EB74E-094A-43F3-8B1C-47BC7F6FECC3@grondar.org>  <1414934579.17308.248.camel@revolution.hippie.lan>  <6FB65828-6A79-4BDE-A9F7-BC472BA538CE@grondar.org>  <CAJ-VmomeOwE3LOpehhJ__G=FCoBDRXrrn%2BSfjwPFODts6YYHNQ@mail.gmail.com>  <20141102192057.GB53947@kib.kiev.ua> <29A795E1-19E2-49E4-9653-143D3F6F3F12@grondar.org> <20141102194625.GC53947@kib.kiev.ua> <751CD860-95B9-4F68-AE69-976B42823AD0@grondar.org> <54568E41.8030305@delphij.net> <20141102201331.GE53947@kib.kiev.ua>  <545693B4.8030602@delphij.net> <1414961583.1200.27.camel@revolution.hippie.lan> <5456A47E.2030405@delphij.net> <5456A69C.6040902@delphij.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 2 Nov 2014, Xin Li wrote:

> Another revision to make Jilles happy -- changed 'block' to 'randrd'
   and 'randwr', I saw his email but forgot to make the change.

% 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

Lots of style bugs (long lines, use of the __func__ obfuscation, and general
verboseness).

% ...
% @@ -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, "randrd", 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);

More style bugs (long lines).  Worse than above since line splitting is
not avoided, giving 1 line 2-3 characters too long and 1 short line.

% @@ -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. */

This adjustment has no effect.  Upper layers already do it for these 2
errors and one more.  Other cases remain broken.

It is simpler to do nothing.  Let upper layers get it right or wrong.
Doesn't matter much either way.


% +

More style bugs.  Extra blank line here.  Space bereofe parentheses above.

%  	}
% -
%  	sx_sunlock(&random_adaptors_lock);
% 
% +	free(random_buf, M_ENTROPY);
% +

Extra blank lines are sprinkled randomly.  Mostly added, but one was removed.
There is an extra blank line after the sx_slock() that separates it from
the blocks(s) of code that it locks; the removed one used the same style.

%  	return (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, "randwr", 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. */

This adjustment has no effect, as above.

It has 1 more style bug than above (line splitting was needed above, but
not here).

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141103115402.H3149>