linux-kernel - Re: [PATCH v2 2/4] driver core: enable drivers to use deferred probe from init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140729231422.GA14494@kroah.com>
Date:	Tue, 29 Jul 2014 16:14:22 -0700
From:	Greg KH <gregkh@...uxfoundation.org>
To:	"Luis R. Rodriguez" <mcgrof@...e.com>
Cc:	Santosh Rastapur <santosh@...lsio.com>,
	Hariprasad S <hariprasad@...lsio.com>,
	Takashi Iwai <tiwai@...e.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Joseph Salisbury <joseph.salisbury@...onical.com>,
	Kay Sievers <kay@...y.org>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Tim Gardner <tim.gardner@...onical.com>,
	Pierre Fersing <pierre-fersing@...rref.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Benjamin Poirier <bpoirier@...e.de>,
	Nagalakshmi Nandigama <nagalakshmi.nandigama@...gotech.com>,
	Praveen Krishnamoorthy <praveen.krishnamoorthy@...gotech.com>,
	Sreekanth Reddy <sreekanth.reddy@...gotech.com>,
	Abhijit Mahajan <abhijit.mahajan@...gotech.com>,
	MPT-FusionLinux.pdl@...gotech.com,
	Linux SCSI List <linux-scsi@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 2/4] driver core: enable drivers to use deferred probe
 from init

On Mon, Jul 28, 2014 at 06:13:43PM -0700, Luis R. Rodriguez wrote:
> >> > Why not just put the initial "register the device" in a single-shot
> >> > workqueue or thread or something like that so that modprobe returns
> >> > instantly back with a success and all is fine?
> >>
> >> That surely is possible but why not a general solution for such kludges?
> >
> > Because the driver should be fixed.  How hard would it be to do what I
> > just suggested here, 20 lines added at most?
> 
> I appreciate the feedback, but don't look at me, I'd happy to go nutty
> on ripping things apart from hairy drivers, however Chelsie folks
> expressed concerns over major rework on the driver... so even if we
> did have something I expect things to take a while to bake / gain
> confidence from others.

"rework"?  Heh, here's a patch that adds 10 lines to the mptsas driver
that should also work for any other driver as well.  Why not just do
this instead?

> This also just addresses this *one* Ethernet driver, there was that
> SCSI driver that created the original report -- Canonical merged
> Joseph's fix as a work around,

What fix was that?

> there is no general solution for this yet, and again with that work
> around you won't find which drivers run into this issue.

Great, fix them as they are found, that's fine.  Again, don't add stuff
to the driver core to paper over crappy drivers, I'm not going to take
that type of change.  I _only_ took the defer binding code as there was
no other way for an individual driver to deal with things if it's
resources were not present yet due to binding order in the system.

So, anyone care to test the patch below on a system that has this
problem to let me know if it would work or not?  Odds are, this should
be a workqueue, to make it cleaner, but a kthread is just so easy to
use...

thanks,

greg k-h

diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 711fcb5cec87..4fd4f36a2d9e 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -51,6 +51,7 @@
 #include <linux/jiffies.h>
 #include <linux/workqueue.h>
 #include <linux/delay.h>	/* for mdelay */
+#include <linux/kthread.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -5393,8 +5394,7 @@ static struct pci_driver mptsas_driver = {
 #endif
 };
 
-static int __init
-mptsas_init(void)
+static int mptsas_real_init(void *data)
 {
 	int error;
 
@@ -5429,9 +5429,19 @@ mptsas_init(void)
 	return error;
 }
 
+static struct task_struct *init_thread;
+
+static int __init
+mptsas_init(void)
+{
+	init_thread = kthread_run(mptsas_real_init, NULL, "mptsas_init");
+	return 0;
+}
+
 static void __exit
 mptsas_exit(void)
 {
+	kthread_stop(init_thread);
 	pci_unregister_driver(&mptsas_driver);
 	sas_release_transport(mptsas_transport_template);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/