Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 21 Feb 2020 19:41:33 -0500
From:   "Theodore Y. Ts'o" <tytso@mit.edu>
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc:     Tony Luck <tony.luck@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] random: always use batched entropy for
 get_random_u{32,64}
Message-ID: <20200222004133.GC873427@mit.edu>
References: <20200216161836.1976-1-Jason@zx2c4.com>
 <20200216182319.GA54139@kroah.com>
 <CA+8MBbKScktNPWPgMqexp9gSX+y2FVnXTDJyyEMVsdONPBpFrQ@mail.gmail.com>
 <CA+8MBbKyRhipHsxb0nvV11Bvv8ypQ_gq5JR8ihfuG6JfBTnxZw@mail.gmail.com>
 <CAHmME9q1rnD5z2bENYhqnM5-XCD+E68nm2RrGRWXt8ntpvfezg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHmME9q1rnD5z2bENYhqnM5-XCD+E68nm2RrGRWXt8ntpvfezg@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Feb 21, 2020 at 09:08:19PM +0100, Jason A. Donenfeld wrote:
> On Thu, Feb 20, 2020 at 11:29 PM Tony Luck <tony.luck@gmail.com> wrote:
> >
> > Also ... what's the deal with a spin_lock on a per-cpu structure?
> >
> >         batch = raw_cpu_ptr(&batched_entropy_u64);
> >         spin_lock_irqsave(&batch->batch_lock, flags);
> >         if (batch->position % ARRAY_SIZE(batch->entropy_u64) == 0) {
> >                 extract_crng((u8 *)batch->entropy_u64);
> >                 batch->position = 0;
> >         }
> >         ret = batch->entropy_u64[batch->position++];
> >         spin_unlock_irqrestore(&batch->batch_lock, flags);
> >
> > Could we just disable interrupts and pre-emption around the entropy extraction?
> 
> Probably, yes... We can address this in a separate patch.

No, we can't; take a look at invalidate_batched_entropy(), where we
need invalidate all of per-cpu batched entropy from a single CPU after
we have initialized the the CRNG.

Since most of the time after CRNG initialization, the spinlock for
each CPU will be on that CPU's cacheline, the time to take and release
the spinlock is not going to be material.

					- Ted