diff mbox

[v3,2/3] cxgb4: use module_long_probe_init()

Message ID 1407882507-325-3-git-send-email-mcgrof@do-not-panic.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Luis R. Rodriguez Aug. 12, 2014, 10:28 p.m. UTC
From: "Luis R. Rodriguez" <mcgrof@suse.com>

cxgb4 probe can take up to over 1 minute when the firmware is
is written and installed on the device, even after this the device
driver still does some device probing and can take quite a bit.
This driver needs fixing but right now it simply wont' work on
some systems. Use the new module_long_probe_init() to annotate
this driver's probe is broken and require some love, but makes
the driver operational until that is fixed.

Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Joseph Salisbury <joseph.salisbury@canonical.com>
Cc: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>
Cc: Tim Gardner <tim.gardner@canonical.com>
Cc: Pierre Fersing <pierre-fersing@pierref.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Benjamin Poirier <bpoirier@suse.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Nagalakshmi Nandigama <nagalakshmi.nandigama@avagotech.com>
Cc: Praveen Krishnamoorthy <praveen.krishnamoorthy@avagotech.com>
Cc: Sreekanth Reddy <sreekanth.reddy@avagotech.com>
Cc: Abhijit Mahajan <abhijit.mahajan@avagotech.com>
Cc: Hariprasad S <hariprasad@chelsio.com>
Cc: Santosh Rastapur <santosh@chelsio.com>
Cc: MPT-FusionLinux.pdl@avagotech.com
Cc: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Anish Bhatt Aug. 13, 2014, 11:33 p.m. UTC | #1
Adding Casey who's actually incharge of this code and missing from the CC list
-Anish
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Casey Leedom Aug. 14, 2014, 4:42 p.m. UTC | #2
On 08/13/2014 04:33 PM, Anish Bhatt wrote:
> Adding Casey who's actually incharge of this code and missing from the CC list

   Thanks Anish!

   As I mentioned to Anish, there are fundamentally two problems here in 
the time being consumed by the cxgb4 PCI probe() function:

  1. When various firmware files aren't present, request_firmware()
     can take a very long time.  This is easily solved by using
     request_firmware_direct() and I certainly have no objection to that.

  2. When there are multiple adapters present in a system which
     need firmware downloaded, each one individually may not take
     a ton of time but together they can exceed simple Module Load
     Timeouts.  There's not a simple answer here.

   Part of the problem here is that it's a Module Load Timeout instead 
of a per-device Probe Timeout.   Part of the problem is that the current 
architecture has Device Probe happening out of the Module Initialization 
when we call pci_register_driver() with our PCI Device ID Table.

   Running the Device Probes asynchronously has been discussed but that 
has the problem that it's then impossible to return the Device Probe 
Status.  This is a problem for Driver Fallback and, if the probe fails, 
we're not supposed to call the Device Remove function. To make this 
work, the synchronous/asynchronous boundary would really need to be up 
in the PCI Infrastructure layer so the Device Probe status could be 
captured in the normal logic.  This would be a moderately large change 
there ...

   Deferring the Device Initialization till the first "ifup" has also 
been discussed and is certainly possible, though a moderately large 
architectural change to every driver which needs it.  It also has the 
unfortunate effect of introducing random large delays directly on user 
commands.  From a User Experience perspective I would tend to want such 
large delays in the Device Probe.  But that's something that really 
deserves a real User Interaction study rather than throwing a dart.

   On the whole, I think that introducing these Module Load Timeouts 
hasn't been well thought out with respect to the repercussions and I'd 
be more inclined to back that out till a well thought out design is 
developed.  But I'm here for the discussion.

Casey
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 36ebbda..5d8231d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -34,6 +34,7 @@ 
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/kthread.h>
 #include <linux/bitmap.h>
 #include <linux/crc32.h>
 #include <linux/ctype.h>
@@ -6815,5 +6816,5 @@  static void __exit cxgb4_cleanup_module(void)
 	destroy_workqueue(workq);
 }
 
-module_init(cxgb4_init_module);
-module_exit(cxgb4_cleanup_module);
+module_long_probe_init(cxgb4_init_module);
+module_long_probe_exit(cxgb4_cleanup_module);