Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751685AbdF2XA6 (ORCPT ); Thu, 29 Jun 2017 19:00:58 -0400 Received: from mail.kernel.org ([198.145.29.99]:47056 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751532AbdF2XA4 (ORCPT ); Thu, 29 Jun 2017 19:00:56 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 66B4F22BE5 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mcgrof@kernel.org MIME-Version: 1.0 In-Reply-To: <20170629155323.06445ddf@cakuba.netronome.com> References: <20170629133530.GA14747@kroah.com> <20170629174046.GC3954@linux-80c1.suse> <20170629183339.GD3954@linux-80c1.suse> <20170629194015.GQ21846@wotan.suse.de> <20170629194455.GR21846@wotan.suse.de> <20170629135822.366fd67a@cakuba.netronome.com> <20170629225003.GU21846@wotan.suse.de> <20170629155323.06445ddf@cakuba.netronome.com> From: "Luis R. Rodriguez" Date: Thu, 29 Jun 2017 16:00:32 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 2/4] swait: add the missing killable swaits To: Jakub Kicinski Cc: Nicolas Broeking , Linus Torvalds , Davidlohr Bueso , Thomas Gleixner , Greg KH , Martin Fuzzey , "Eric W. Biederman" , Dmitry Torokhov , Daniel Wagner , David Woodhouse , jewalt@lgsinnovations.com, rafal@milecki.pl, Arend Van Spriel , "Rafael J. Wysocki" , "Li, Yi" , atull@kernel.org, Moritz Fischer , Petr Mladek , Johannes Berg , Emmanuel Grumbach , "Coelho, Luciano" , Kalle Valo , Andrew Lutomirski , Kees Cook , "AKASHI, Takahiro" , David Howells , Peter Jones , Hans de Goede , Alan Cox , "Theodore Ts'o" , Michael Kerrisk , Paul Gortmaker , Marcelo Tosatti , Matthew Wilcox , Linux API , linux-fsdevel , Linux Kernel Mailing List , "stable # 4 . 6" 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: 2571 Lines: 62 On Thu, Jun 29, 2017 at 3:53 PM, Jakub Kicinski wrote: > On Fri, 30 Jun 2017 00:50:03 +0200, Luis R. Rodriguez wrote: >> On Thu, Jun 29, 2017 at 01:58:22PM -0700, Jakub Kicinski wrote: >> > On Thu, 29 Jun 2017 21:44:55 +0200, Luis R. Rodriguez wrote: >> > > > Since this swake_up() --> swake_up_all() reportedly *fixed* the one wake up >> > > > issue it would seem this does queue [0]. That said, I don't see any simple tests >> > > > tools/testing/selftests/swait but then again we don't have test for regular >> > > > waits either... >> > > > >> > > > [0] https://bugzilla.kernel.org/show_bug.cgi?id=195477 >> > > >> > > I should also note that the swake_up_all() should have only helped in cases where >> > > 3 cards were used, as if only 2 were used that should have been covered by just >> > > the swake_up(). Unless of course I hear otherwise by the reporter, Nicolas or >> > > from Jakub. >> > >> > I was hitting this with 2 cards. >> >> Thanks! >> >> Thing is I'm not convinced the issue with 2 cards was the swake_up() Vs >> swake_up_all() in this case though. Let's recall also the missing wake up on >> errors! And the fact that netronome has optional firmware, which naturally can >> fail. >> >> So could the issue with 2 cards instead of the miss of a wake up on error due >> to batched requests ? If so then that still would not put blame on the >> swake_up()! > > Sorry, that was during manual tests. I had the driver request the > firmware with _nowait() without an uevent, Can you provide the output of grep CONFIG_FW_LOADER_USER_HELPER .config I'd like to see if CONFIG_FW_LOADER_USER_HELPER_FALLBACK was disabled. Not to be confused with the CONFIG_FW_LOADER_USER_HELPER. When the uevent is not set its also known as "a custom fallback mechanism" and by default that has set a timeout of MAX_SCHEDULE_TIMEOUT, which means that even if the direct filesystem lookup failed the fallback mechanism would be used and just sit and wait for what seems to be forever until user input was provided. > and then after I manually > wrote -1 to loading only one would get woken up. Great, this helps, so for those not familiar with the firmware sysfs fallback mechanism: The relevant values one could use are: 1: Start a load, discarding any previous partial load. 0: Conclude the load and hand the data to the driver code. -1: Conclude the load with an error and discard any written data. So you are triggering a failure. And indeed I expect the patch I just provided to be the one to fix your issue with 2 cards. Luis