Re: [2.6 patch] disallow unloading of ibmphp
From: Adrian Bunk
Date: Fri Mar 17 2006 - 15:27:59 EST
On Fri, Mar 17, 2006 at 09:20:09PM +0100, Adrian Bunk wrote:
> On Tue, Mar 14, 2006 at 04:21:04PM -0800, Stephen Hemminger wrote:
> > On Tue, 14 Mar 2006 16:02:12 -0800
> > Greg KH <greg@xxxxxxxxx> wrote:
> >
> > > On Wed, Mar 15, 2006 at 09:47:00AM +1100, Srihari Vijayaraghavan wrote:
> > > > Before (in 2.6.16-rc*):
> > > > $ egrep 'ibmphp' /proc/modules
> > > > ibmphp 67809 4294967295 - Live 0xf8910000
> > > > ^^^^^^^^^^
> > > >
> > > > After [1]:
> > > > ibmphp 64224 0 - Live 0xf8965000
> > > > ^
> > > >
> > > > Of course, now I'm able to successfully unload ibmphp
> > > > (& subsequently load it too :)) without any
> > > > observeable problems.
> > > >
> > > > It'd seem, thro struct hotplug_slot_ops, module ref
> > > > count for ibmphp is taken care of. No?
> > >
> > > No. I don't think this driver likes to be unloaded due to the
> > > instability of the hardware if that happens. So let's just let it not
> > > be unloaded, and hope that the hardware can die in peace and never get
> > > put into any new machines...
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > The proper way to prevent unloading is just not to have a module exit routine,
> > rather than causing ref count to be off. The the module subsystem will
> > mark it as unsafe to unload. Unless it wants to allow unsafe forced unload.
> > But IMHO either it needs to be safe to unload or not allow it.
> >...
>
>
> What about the patch below that also adds a comment and removes some
> more code?
>
> I'll send a "diff -uwp" for better readability of the ibmphp_hpc.c
> changes in a reply.
It's below.
cu
Adrian
--- linux-2.6.16-rc6-mm1-full/drivers/pci/hotplug/ibmphp_core.c.old 2006-03-17 20:51:09.000000000 +0100
+++ linux-2.6.16-rc6-mm1-full/drivers/pci/hotplug/ibmphp_core.c 2006-03-17 21:00:52.000000000 +0100
@@ -736,7 +736,7 @@ static struct pci_func *ibm_slot_find(u8
* the pointers to pci_func, bus, hotplug_slot, controller,
* and deregistering from the hotplug core
*************************************************************/
-static void free_slots(void)
+static __init void free_slots(void)
{
struct slot *slot_cur;
struct list_head * tmp;
@@ -1328,7 +1328,7 @@ struct hotplug_slot_ops ibmphp_hotplug_s
*/
};
-static void ibmphp_unload(void)
+static void __init ibmphp_unload(void)
{
free_slots();
debug("after slots\n");
@@ -1398,10 +1398,6 @@ static int __init ibmphp_init(void)
goto error;
}
- /* lock ourselves into memory with a module
- * count of -1 so that no one can unload us. */
- module_put(THIS_MODULE);
-
exit:
return rc;
@@ -1410,13 +1406,11 @@ error:
goto exit;
}
-static void __exit ibmphp_exit(void)
-{
- ibmphp_hpc_stop_poll_thread();
- debug("after polling\n");
- ibmphp_unload();
- debug("done\n");
-}
-
module_init(ibmphp_init);
-module_exit(ibmphp_exit);
+
+/* Greg KH <greg@xxxxxxxxx>:
+ * I don't think this driver likes to be unloaded due to the
+ * instability of the hardware if that happens. So let's just let it not
+ * be unloaded, and hope that the hardware can die in peace and never get
+ * put into any new machines...
+ */
--- linux-2.6.16-rc6-mm1-full/drivers/pci/hotplug/ibmphp.h.old 2006-03-17 20:52:49.000000000 +0100
+++ linux-2.6.16-rc6-mm1-full/drivers/pci/hotplug/ibmphp.h 2006-03-17 20:52:55.000000000 +0100
@@ -398,7 +398,6 @@ extern int ibmphp_hpc_writeslot (struct
extern void ibmphp_lock_operations (void);
extern void ibmphp_unlock_operations (void);
extern int ibmphp_hpc_start_poll_thread (void);
-extern void ibmphp_hpc_stop_poll_thread (void);
//----------------------------------------------------------------------------
--- linux-2.6.16-rc6-mm1-full/drivers/pci/hotplug/ibmphp_hpc.c.old 2006-03-17 20:53:02.000000000 +0100
+++ linux-2.6.16-rc6-mm1-full/drivers/pci/hotplug/ibmphp_hpc.c 2006-03-17 20:58:49.000000000 +0100
@@ -101,12 +101,10 @@ static int to_debug = FALSE;
//----------------------------------------------------------------------------
// global variables
//----------------------------------------------------------------------------
-static int ibmphp_shutdown;
static int tid_poll;
static struct mutex sem_hpcaccess; // lock access to HPC
static struct semaphore semOperations; // lock all operations and
// access to data structures
-static struct semaphore sem_exit; // make sure polling thread goes away
//----------------------------------------------------------------------------
// local function prototypes
//----------------------------------------------------------------------------
@@ -135,9 +133,7 @@ void __init ibmphp_hpc_initvars (void)
mutex_init(&sem_hpcaccess);
init_MUTEX (&semOperations);
- init_MUTEX_LOCKED (&sem_exit);
to_debug = FALSE;
- ibmphp_shutdown = FALSE;
tid_poll = 0;
debug ("%s - Exit\n", __FUNCTION__);
@@ -833,10 +829,6 @@ static void poll_hpc (void)
debug ("%s - Entry\n", __FUNCTION__);
- while (!ibmphp_shutdown) {
- if (ibmphp_shutdown)
- break;
-
/* try to get the lock to do some kind of hardware access */
down (&semOperations);
@@ -896,9 +888,6 @@ static void poll_hpc (void)
up (&semOperations);
msleep(POLL_INTERVAL_SEC * 1000);
- if (ibmphp_shutdown)
- break;
-
down (&semOperations);
if (poll_count >= POLL_LATCH_CNT) {
@@ -912,8 +901,6 @@ static void poll_hpc (void)
up (&semOperations);
/* sleep for a short time just for good measure */
msleep(100);
- }
- up (&sem_exit);
debug ("%s - Exit\n", __FUNCTION__);
}
@@ -1094,37 +1081,6 @@ int __init ibmphp_hpc_start_poll_thread
}
/*----------------------------------------------------------------------
-* Name: ibmphp_hpc_stop_poll_thread
-*
-* Action: stop polling thread and cleanup
-*---------------------------------------------------------------------*/
-void __exit ibmphp_hpc_stop_poll_thread (void)
-{
- debug ("%s - Entry\n", __FUNCTION__);
-
- ibmphp_shutdown = TRUE;
- debug ("before locking operations \n");
- ibmphp_lock_operations ();
- debug ("after locking operations \n");
-
- // wait for poll thread to exit
- debug ("before sem_exit down \n");
- down (&sem_exit);
- debug ("after sem_exit down \n");
-
- // cleanup
- debug ("before free_hpc_access \n");
- free_hpc_access ();
- debug ("after free_hpc_access \n");
- ibmphp_unlock_operations ();
- debug ("after unlock operations \n");
- up (&sem_exit);
- debug ("after sem exit up\n");
-
- debug ("%s - Exit\n", __FUNCTION__);
-}
-
-/*----------------------------------------------------------------------
* Name: hpc_wait_ctlr_notworking
*
* Action: wait until the controller is in a not working state
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/