Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752042AbdIMTas (ORCPT ); Wed, 13 Sep 2017 15:30:48 -0400 Received: from mail-io0-f181.google.com ([209.85.223.181]:43600 "EHLO mail-io0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751308AbdIMTap (ORCPT ); Wed, 13 Sep 2017 15:30:45 -0400 X-Google-Smtp-Source: ADKCNb5T0u81FgSUaCAfmpwy4h789Q8KvzS0TVU37/3MXuCeOErpUqWuEMQ+Bh/QYXXzxAd7blcXvFXviZ9g3xu5fo0= MIME-Version: 1.0 In-Reply-To: <20170913183809.GN16216@wotan.suse.de> References: <20170912165308.904472972@linuxfoundation.org> <20170912165310.407586301@linuxfoundation.org> <20170912172008.GF16216@wotan.suse.de> <20170913004758.GA21439@kroah.com> <20170913183809.GN16216@wotan.suse.de> From: Linus Torvalds Date: Wed, 13 Sep 2017 12:30:44 -0700 X-Google-Sender-Auth: mWC-HZLynugd6pgicQ1UU7Du88w Message-ID: Subject: Re: [PATCH 4.13 20/27] Revert "firmware: add sanity check on shutdown/suspend" To: "Luis R. Rodriguez" Cc: "Rafael J. Wysocki" , Marcel Holtmann , Greg Kroah-Hartman , Linux Kernel Mailing List , stable , Gabriel C , Arend Van Spriel Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1981 Lines: 48 On Wed, Sep 13, 2017 at 11:38 AM, Luis R. Rodriguez wrote: >> Commit 06a45a93e7d34a seems to be a cleanup. The arguments in >> 06a45a93e7d3 ("firmware: move umh try locks into the umh code") seem >> valid, and there's no real reason to worry about that FW_OPT_NOWAIT >> etc for the direct-from-filesystem loading. That's simply not >> sensible. > > Indeed! That stupid UMH lock *seems* wrong on the direct filesystem path! > > Hence these changes. > > The devil is in the details though. That UMH lock however carried an implicit > suspend guard, the "cleanup" actually then has a functional change. The commit > which was reverted provided the safe guard in generic form, in case we already > had become dependent on the suspend guard. This UMH lock on the direct FS path > then added an implicit "arbitrary rule", as you put it, on the firmware API. I still refuse to revert a commit "just because". It improved the code. There is no actual sign that it causes problems. Yes, moving the lock may change behavior, but nothing you say actually makes me think it regressed anything. I refuse to do this "let's just go back to what we used to have, without an actual _reason_ to go back to it". The old user-mode-helper crap was crap that had its own totally separate issues. Even without the problems that we had with incredibly bad maintainership of the user mode side, the usermode helper had serious issues with just the fact that user mode itself is disabled by the suspend/resume process. So getting rid of the locking that was added for usermode helper ion the direct file loading path makes a lot of sense. Just saying that "we tried to re-introduce that locking in another form, and that was a mistake, and got reverted" is *not* a reason to then revert the movement. Yes, the movement of the locking might need to be reverted too - but only if it actually shows problems. We should *not* go back to old code "just because". Linus