linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Prarit Bhargava <prarit@redhat.com>
To: Ming Lei <ming.lei@canonical.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	x86@kernel.org,
	Andreas Herrmann <herrmann.der.user@googlemail.com>,
	tigran@aivazian.fsnet.co.uk
Subject: Re: [PATCH 1/2] firmware, fix request_firmware_nowait() freeze with no uevent
Date: Wed, 23 Oct 2013 08:02:47 -0400	[thread overview]
Message-ID: <5267BAE7.2090300@redhat.com> (raw)
In-Reply-To: <5267A6CA.1000505@redhat.com>



On 10/23/2013 06:36 AM, Prarit Bhargava wrote:
> 
> 
> On 10/23/2013 12:16 AM, Ming Lei wrote:
>> On Wed, Oct 23, 2013 at 7:15 AM, Prarit Bhargava <prarit@redhat.com> wrote:
>>> On 10/21/2013 10:35 PM, Ming Lei wrote:
>>>>
>>>> That is why NOHOTPLUG isn't encouraged to be taken, actually
>>>> I don't suggest you to do that too, :-)
>>> Okay ... I can certainly switch to HOTPLUG.
>>
>> OK, that should be the right approach.
>>
>>>
>>>>
>>>> You need to make sure your approach won't break micro-code
>>>> update application in current/previous distributions.
>>>
>>> I've tested the following distributions today on a Dell PE 1850:  Ubuntu, SuSe,
>>> Linux Mint, and of course Fedora.  I do not see any issues with either the
>>> microcode update or the dell_rbu driver.  Unfortunately I do not have access to
>>
>> Actually I am wondering if your tests are enough because kernel
>> can't break user-space, which means lots of previous old version
>> distributions should surely be covered, :-)
> 
> I've tested an old version of Suse and a few older RHEL versions for kicks.  No
> problems.  I'm testing an older version of Ubuntu ATM and will update with
> details (it doesn't look like it does anything different FWIW so I'm not worried).
> 
>>
>> If you keep HOTPLUG, only change to request_firmware_nowait(),
>> that should be OK since the loading protocol between userspace and
>> kernel won't change wrt. microcode updating.
>>
>>> a system that uses the lattice-ecp3-config, however, from code inspection it
>>> looks like the driver looks at a specific place for the FW update and then
>>> applies it via the call function in request_firmware_nowait() so it looks like
>>> it is solid too.
>>>
>>> I think maybe this patchset should be split into two separate submits, one for
>>> the microcode and the second to figure out if the code really should wait
>>> indefinitely.  AFAICT neither use case in the kernel expects an indefinite wait.
>>
>> If you switch to HOTPLUG, you needn't worry about waiting indefinitely,
>> need you?
> 
> Nope ... I'll modify the code and retest.  

After all this I completely forgot the problem I'm trying to solve here.  The
issue is that with HOTPLUG & request_microcode_nowait(), if the microcode image
is not found (that is the file is not found on disk), then EACH cpu waits 1
minute and it takes 2 hours for a 120 cpu box to load the microcode module.

Which is terrible... so HOTPLUG doesn't work here.

Let me back up Ming and see if you have a better solution for me.  I have a
system that does not have the x86 microcode loaded on disk.  I use the microcode
module which calls request_firmware_nowait() to load the microcode image and I
want it done as fast as possible.  The microcode loader does not have a uevent
so I'm not waiting on userspace for completion.

How do I avoid the 60 second delay/cpu introduced in the microcode path?  I
don't see one.  If I use HOTPLUG I'm waiting 60 seconds.  If I use NOHOTPLUG
AFAICT the loading function never will return.  AFAICT the same issue arises
with the dell_rbu code -- it appears to never load the dell_rbu firmware.

P.

  reply	other threads:[~2013-10-23 12:03 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-20 21:35 [PATCH 0/2] Improve firmware loading times on AMD and Intel Prarit Bhargava
2013-10-20 21:35 ` [PATCH 1/2] firmware, fix request_firmware_nowait() freeze with no uevent Prarit Bhargava
2013-10-21 12:24   ` Ming Lei
2013-10-21 22:24     ` Prarit Bhargava
2013-10-22  2:35       ` Ming Lei
2013-10-22 23:15         ` Prarit Bhargava
2013-10-23  4:16           ` Ming Lei
2013-10-23 10:36             ` Prarit Bhargava
2013-10-23 12:02               ` Prarit Bhargava [this message]
2013-10-23 13:21                 ` Ming Lei
2013-10-23 14:08                   ` Prarit Bhargava
2013-10-24  1:54                     ` Ming Lei
2013-10-24 11:17                 ` Henrique de Moraes Holschuh
2013-10-24 12:05                   ` Prarit Bhargava
2013-10-20 21:35 ` [PATCH 2/2] intel_microcode, Fix long microcode load time when firmware file is missing Prarit Bhargava
2013-10-21 12:20   ` Ming Lei
2013-10-21 12:26     ` Prarit Bhargava
2013-10-21 12:32       ` Ming Lei
2013-10-21 14:25         ` Prarit Bhargava
2013-10-22  2:43           ` Ming Lei
2013-10-22 23:16             ` Prarit Bhargava
2013-10-20 22:58 ` [PATCH 0/2] Improve firmware loading times on AMD and Intel Andi Kleen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5267BAE7.2090300@redhat.com \
    --to=prarit@redhat.com \
    --cc=herrmann.der.user@googlemail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@canonical.com \
    --cc=tigran@aivazian.fsnet.co.uk \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).