From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752560AbbI3Ok3 (ORCPT <rfc822;w@1wt.eu>);
	Wed, 30 Sep 2015 10:40:29 -0400
Received: from mail-la0-f48.google.com ([209.85.215.48]:34083 "EHLO
	mail-la0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751226AbbI3Ok1 (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 30 Sep 2015 10:40:27 -0400
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Andi Kleen <andi@firstfloor.org>
Cc: tytso@mit.edu, linux-kernel@vger.kernel.org,
        Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH 1/3] Make /dev/urandom scalable
Organization: D03
References: <1443115148-13937-1-git-send-email-andi@firstfloor.org>
	<1443115148-13937-2-git-send-email-andi@firstfloor.org>
X-Hashcash: 1:20:150930:andi@firstfloor.org::sm8A4W2OlDQnAhoh:00000000000000000000000000000000000000000004Ae
X-Hashcash: 1:20:150930:ak@linux.intel.com::BWBYtW7W8k3V0xJd:00000000000000000000000000000000000000000000/sA
X-Hashcash: 1:20:150930:tytso@mit.edu::M1oCaH1zqueDMgTk:00001s3Y
X-Hashcash: 1:20:150930:linux-kernel@vger.kernel.org::6DzbzsNT7r+JG72L:0000000000000000000000000000000007Ap1
Date: Wed, 30 Sep 2015 16:40:23 +0200
In-Reply-To: <1443115148-13937-2-git-send-email-andi@firstfloor.org> (Andi
	Kleen's message of "Thu, 24 Sep 2015 10:19:06 -0700")
Message-ID: <874mickm20.fsf@rasmusvillemoes.dk>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Sep 24 2015, Andi Kleen <andi@firstfloor.org> wrote:

>
> v2: Fix name of pool 0. Fix race with interrupts. Make
> iteration loops slightly more efficient. Add ifdefs to avoid
> any extra code on non-NUMA. Delay other pool use to when
> the original pool initialized and initialize the pools from
> pool 0. Add comments on memory allocation.

More bikeshedding. Please ignore unless you do a v3 anyway.

>  static struct entropy_store nonblocking_pool = {
>  	.poolinfo = &poolinfo_table[1],
> -	.name = "nonblocking",
> +	.name = "nonblocking pool 0",

Hm, yeah, but then you should probably also update the blocking pool's
name (or use the format "nonblocking %d").

> +#define POOL_INIT_BYTES (128 / 8)
> +
> +void init_node_pools(void)
> +{
> +#ifdef CONFIG_NUMA
> +	int i;
> +	for (i = 0; i < num_possible_nodes(); i++) {

int num_nodes = num_possible_nodes();
for (i = 0; i < num_nodes; i++) {

>  static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
>  {
> +	int i;
>  	int size, ent_count;
>  	int __user *p = (int __user *)arg;
>  	int retval;
> +	struct entropy_store *pool;
>  
>  	switch (cmd) {
>  	case RNDGETENTCNT:
> @@ -1569,6 +1696,10 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
>  			return -EPERM;
>  		input_pool.entropy_count = 0;
>  		nonblocking_pool.entropy_count = 0;
> +		if (nonblocking_node_pool)
> +			for_each_nb_pool (i, pool) {
> +				pool->entropy_count = 0;
> +			} end_for_each_nb();

extra ;. harmless in this case, but could cause slightly hard-to-understand build
failures if that if grows an else.

Rasmus