From owner-svn-src-head@FreeBSD.ORG Sat Jan 28 07:47:58 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 820FD106566B; Sat, 28 Jan 2012 07:47:58 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail08.syd.optusnet.com.au (mail08.syd.optusnet.com.au [211.29.132.189]) by mx1.freebsd.org (Postfix) with ESMTP id 147618FC13; Sat, 28 Jan 2012 07:47:57 +0000 (UTC) Received: from c211-30-171-136.carlnfd1.nsw.optusnet.com.au (c211-30-171-136.carlnfd1.nsw.optusnet.com.au [211.30.171.136]) by mail08.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q0S7lo4k016202 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 28 Jan 2012 18:47:54 +1100 Date: Sat, 28 Jan 2012 18:47:50 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andrey Chernov In-Reply-To: <20120128045854.GA12556@vniz.net> Message-ID: <20120128180310.F1114@besplex.bde.org> References: <20120126143819.GA88677@vniz.net> <20120126155626.GA92229@vniz.net> <201201261132.38320.jhb@freebsd.org> <20120126165521.GA92622@vniz.net> <20120126175021.GA93016@vniz.net> <20120127201855.K1604@besplex.bde.org> <20120128043118.GA12241@vniz.net> <20120128045854.GA12556@vniz.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: src-committers@freebsd.org, John Baldwin , svn-src-all@freebsd.org, Bruce Evans , svn-src-head@freebsd.org, David Schultz , Mark Murray Subject: Re: svn commit: r230230 - head/sys/dev/random X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 28 Jan 2012 07:47:58 -0000 On Sat, 28 Jan 2012, Andrey Chernov wrote: > New verson addressed bde's style things: Thanks, but ... > --- sys/libkern.h.old 2012-01-16 07:15:12.000000000 +0400 > +++ sys/libkern.h 2012-01-28 08:49:19.000000000 +0400 > @@ -70,6 +70,11 @@ static __inline int abs(int a) { return > static __inline long labs(long a) { return (a < 0 ? -a : a); } > static __inline quad_t qabs(quad_t a) { return (a < 0 ? -a : a); } > > +#define ARC4_ENTR_NONE 0 /* Don't have entropy yet. */ > +#define ARC4_ENTR_HAVE 1 /* Have entropy. */ > +#define ARC4_ENTR_SEED 2 /* Reseeding. */ > +extern int arc4rand_iniseed_state; > + > /* Prototypes for non-quad routines. */ > struct malloc_type; > uint32_t arc4random(void); ... I will also ask for data and macros to be sorted into the data an macro sections and not unsorted in between the first set of inlines and the prototypes. (Whether the macros should be all in a macro section or near the variables or prototypes that they are for is unclear, especially since the current organization for this in this file is random.) This file was last nearly sorted in FreeBSD-2.1. Now its organization is partly to put macros and data together with inline functions that use them. This gives lots of subsections which are not clearly delimited and is not used in the prototype section, where it would split up that section too much. >From your previous reply (non-style part): > > > +volatile int arc4rand_iniseed_state = ARC4_ENTR_NONE; > > > + > > > > I don't see what all the volatile qualifiers and atomic ops are for. > > The volatiles certainly have no effect, since this variable is only > > accessed by atomic ops which only access variables as volatile whether > > you want this or not. See das's reply for more about the atomic ops. > > Here I think they just close a window that would be open just once for > > each for a few instructions while booting. > > I will remove volatile. > About atomic ops: arc4rand calls are protected with mutex and yarrow > calls are protected, but they can works concurrently wich each > other. So, if we have atomic ops, why not to use them to close one time > window too? Hmm, it's tricky code either way. I use similar cmpsets in sio, and also need a loop to synchronize the final state change. But when the state is seen to be final, atomic ops are not needed to check that it doesn't change (or to see that it is final), since it won't change again. This gives a tiny optimization that would be larger here. This is mostly overkill, since sio is^Wwas normally present at boot time. New-bus and most drivers don't have complications like this, but might need them more for loadable drivers. It is very unclear that new-bus's Giant locking is enough for anything (unless the whole system also uses Giant locking). Bruce