From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752194AbcIIETt (ORCPT ); Fri, 9 Sep 2016 00:19:49 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:53883 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750833AbcIIETr (ORCPT ); Fri, 9 Sep 2016 00:19:47 -0400 MIME-Version: 1.0 In-Reply-To: References: <1473237908-20989-1-git-send-email-wagi@monom.org> <1473237908-20989-2-git-send-email-wagi@monom.org> <20160908201106.GI3296@wotan.suse.de> From: Ming Lei Date: Fri, 9 Sep 2016 12:19:38 +0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 1/4] firmware: Move umh locking code into fw_load_from_user_helper() To: "Luis R. Rodriguez" Cc: "rafael.j.wysocki" , Jeff Mahoney , David Woodhouse , Greg Kroah-Hartman , Bjorn Andersson , Julia Lawall , NeilBrown , Andrew Morton , David Howells , Daniel Wagner , Mimi Zohar , Andy Lutomirski , Fengguang Wu , Linus Torvalds , Daniel Wagner , Frederic Weisbecker , Johannes Berg , Takashi Iwai , Oleg Nesterov , Arend Van Spriel , Linux Kernel Mailing List , Dmitry Torokhov , Daniel Vetter Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 9, 2016 at 11:39 AM, Luis R. Rodriguez wrote: > On Sep 8, 2016 6:22 PM, "Ming Lei" wrote: >> >> On Fri, Sep 9, 2016 at 4:11 AM, Luis R. Rodriguez >> wrote: >> > On Thu, Sep 08, 2016 at 11:37:54PM +0800, Ming Lei wrote: >> >> On Wed, Sep 7, 2016 at 4:45 PM, Daniel Wagner wrote: >> >> > From: Daniel Wagner >> >> > >> >> > When we load the firmware directly we don't need to take the umh >> >> > lock. >> >> >> >> I am wondering if it can be wrong. >> > >> > If you disable the firmware UMH why would we need to lock if the lock is >> > being >> > shown only used for the firmare UMH ? >> > >> >> Actually in case of firmware loading, the usermode helper lock doesn't >> >> only mean the user helper is usable, and it also may serve to mark the >> >> filesystem/block device is ready for firmware loading, and of couse >> >> direct >> >> loading need fs/block to be ready too. >> > >> > Yes but that's a race I've identified a while ago present even if you >> > use initramfs *and* >> > use kernel_read_file_from_path() on any part of the kernel [0], I >> > proposed a possible >> >> Actualy I mean the situation of suspend vs. resume, and some drivers >> still may not benefit from firmware loading cache when requesting loading >> in .resume(), at that time it is still too early for direct loading. >> With UMH lock, >> we can get a warning or avoid the issue. > > Agreed, but that would seem odd and perhaps misleading to have a try lock > for UMH when no firmware UMH code is enabled. This should probably made > clear in comments for now as to why we have it then and we should just mark That is very helpful, :-) > a TODO item to generalize this to a common freezer check. Surprised we don't > have one yet. Rafael ? > > Luis