Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759396Ab3EWOgD (ORCPT ); Thu, 23 May 2013 10:36:03 -0400 Received: from cantor2.suse.de ([195.135.220.15]:60396 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758125Ab3EWOgB (ORCPT ); Thu, 23 May 2013 10:36:01 -0400 Date: Thu, 23 May 2013 16:36:20 +0200 Message-ID: From: Takashi Iwai To: Ming Lei Cc: Dave Jones , "H. Peter Anvin" , Linux Kernel , x86@kernel.org, fenghua.yu@intel.com Subject: Re: microcode loading got really slow. In-Reply-To: References: <20130521230332.GC12713@redhat.com> <519D1668.6000601@zytor.com> <20130522200012.GA15456@redhat.com> <20130523033911.GA9411@redhat.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5570 Lines: 132 At Thu, 23 May 2013 22:28:51 +0800, Ming Lei wrote: > > On Thu, May 23, 2013 at 9:21 PM, Takashi Iwai wrote: > > At Thu, 23 May 2013 21:04:53 +0800, > > Ming Lei wrote: > >> > >> On Thu, May 23, 2013 at 8:05 PM, Takashi Iwai wrote: > >> > At Thu, 23 May 2013 18:45:29 +0800, > >> > Ming Lei wrote: > >> >> > >> >> On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai wrote: > >> >> > > >> >> > No, f/w loader always fall back to user mode helper, as long as its > >> >> > support is built in. And doing that for microcode driver in that code > >> >> > path isn't only superfluous but also broken due to request_firmware > >> >> > call in module init. > >> >> > >> >> Firstly, it is not good to do this since some distributions doesn't support > >> >> direct loading and doesn't have udevd(such as, android). > >> >> > >> >> Secondly, returning failure from request_firmware_direct() doesn't mean > >> >> the firmware doesn't exist since distribution may put the firmware other where. > >> > > >> > Right, the non-standard path is the problem, and basically the only > >> > problem. The distribution that doesn't support the direct loading > >> > means nothing but that. > >> > >> Suppose it is, it is the fact, and it isn't OK to break this distribution. > >> > >> Also udev supports user-defined rules to load firmware, which > >> means some drivers may not put their firmware in the default > >> path of distribution's firmware. > > > > It's why I suggested to put a warning in that path as the first step. > > So we can see whether there is any actual user. > > If you plan to do it, it'd better to add default firmware path of some > distributions into firmware_class.c first, otherwise it may cause > unnecessary noise for this distribution. > > But if more default search paths are added, it might cause mistaken > firmwares found under incorrect path, for example, android's > default path is "/etc/firmware" and "/vendor/firmware"(maybe different > for different versions). > > Also, putting default search paths into kernel isn't good, which was > introduced unwillingly for well-known reason. Maybe we can create a new Kconfig to specify non-standard firmware path? > >> >> Anyway, this example is very specific(no firmware can be accepted), and > >> >> request_firmware_nowait() should be OK for the situation. > >> > > >> > Oh no, rewriting with request_firmware_nowait() should be really the > >> > last choice. It would change the code flow awfully bad in most > >> > cases. > >> > > >> > The new kernel driver has a better firmware mechanism. If it's only > >> > the question of paths, we should move on toward that direction and > >> > drop the too complex old way. I'd vote for a warning shown when a > >> > >> Simply dropping the old way may cause user space regression. > > > > It's already broken :) > > It is different, the current issue is caused by udev, not by kernel, :-) Yeah :) > >> > firmware file is loaded via user mode helper (except for explicit > >> > cases like FW_ACTION_NOHOTPLUG), for example. > >> > >> As it is a very driver specific problem, it is better to solve it inside driver. > > > > Yes, this proposal is basically not meant as a fix for this particular > > issue but rather for future movement in general. > > > >> >> >> wrt. this problem, I think we > >> >> >> need to know why the direct loading is failed. > >> >> > > >> >> > The reason is obvious: the requested f/w file doesn't exist. > >> >> > And it's fine, because the microcode update is an optional operation. > >> >> > If no f/w file is found, it's not handled as an error. It just means > >> >> > that no need to update, continuing to work. > >> >> > >> >> OK, as said above, the example is very specific, and might be > >> >> workarounded by request_firmware_nowait(). > >> > > >> > It's not that easy in this case. The microcode loader driver core > >> > module doesn't invoke request_firmware() directly but it's via cpu > >> > driver. And the same callback is called in different code paths, not > >> > only at init but also via sysfs write. Thus the request_firmware() > >> > call must be synchronous there. > >> > >> I don't think the way is too difficult to implement. In the path which > >> requires synchronization, it can be waited on one completion after > >> calling request_firmware_nowait(). > > > > This sounds already like unnecessary complexity. Also, what if > > concurrent accesses? > > The request_firmware_no_wait() supports concurrent accesses on > either same firmware or not. Yes, but I meant about the synchronization part. Then you'll need multiple waiters. > > Also, I wonder why the kernel needs to be "fixed" for this, if the > > problem is really the stuck in udev. In this regard, we didn't change > > anything from the beginning. There was an implicit "wish", that the > > f/w loading shouldn't be done in the module init, but this has been > > never treated as a golden rule. > > No, there isn't the golden rule, and it is reasonable or inevitable > sometimes to load firmware in module init, for example, I remember some > wireless dongles in which people can't read its mac address without > downloading firmware, that means some devices may not be initialized > successfully without firmware. Right. Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/