On Tue, Mar 14, 2006 at 04:21:04PM -0800, Stephen Hemminger wrote:
> On Tue, 14 Mar 2006 16:02:12 -0800
> Greg KH <[email protected]> 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.
<-- snip -->
The ibmphp driver shouldn't be unloaded.
This patch replaces the module_exit() with a comment why it shouldn't
happen and removes some now no longer needed code.
Signed-off-by: Adrian Bunk <[email protected]>
---
drivers/pci/hotplug/ibmphp.h | 1
drivers/pci/hotplug/ibmphp_core.c | 24 +---
drivers/pci/hotplug/ibmphp_hpc.c | 178 +++++++++++-------------------
3 files changed, 76 insertions(+), 127 deletions(-)
--- 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 <[email protected]>:
+ * 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,87 +829,78 @@ 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);
+ /* try to get the lock to do some kind of hardware access */
+ down (&semOperations);
- switch (poll_state) {
- case POLL_LATCH_REGISTER:
- oldlatchlow = curlatchlow;
- ctrl_count = 0x00;
- list_for_each (pslotlist, &ibmphp_slot_head) {
- if (ctrl_count >= ibmphp_get_total_controllers())
- break;
- pslot = list_entry (pslotlist, struct slot, ibm_slot_list);
- if (pslot->ctrl->ctlr_relative_id == ctrl_count) {
- ctrl_count++;
- if (READ_SLOT_LATCH (pslot->ctrl)) {
- rc = ibmphp_hpc_readslot (pslot,
- READ_SLOTLATCHLOWREG,
- &curlatchlow);
- if (oldlatchlow != curlatchlow)
- process_changeinlatch (oldlatchlow,
- curlatchlow,
- pslot->ctrl);
- }
- }
- }
- ++poll_count;
- poll_state = POLL_SLEEP;
- break;
- case POLL_SLOTS:
- list_for_each (pslotlist, &ibmphp_slot_head) {
- pslot = list_entry (pslotlist, struct slot, ibm_slot_list);
- // make a copy of the old status
- memcpy ((void *) &myslot, (void *) pslot,
- sizeof (struct slot));
- rc = ibmphp_hpc_readslot (pslot, READ_ALLSTAT, NULL);
- if ((myslot.status != pslot->status)
- || (myslot.ext_status != pslot->ext_status))
- process_changeinstatus (pslot, &myslot);
- }
- ctrl_count = 0x00;
- list_for_each (pslotlist, &ibmphp_slot_head) {
- if (ctrl_count >= ibmphp_get_total_controllers())
- break;
- pslot = list_entry (pslotlist, struct slot, ibm_slot_list);
- if (pslot->ctrl->ctlr_relative_id == ctrl_count) {
- ctrl_count++;
- if (READ_SLOT_LATCH (pslot->ctrl))
- rc = ibmphp_hpc_readslot (pslot,
- READ_SLOTLATCHLOWREG,
- &curlatchlow);
+ switch (poll_state) {
+ case POLL_LATCH_REGISTER:
+ oldlatchlow = curlatchlow;
+ ctrl_count = 0x00;
+ list_for_each (pslotlist, &ibmphp_slot_head) {
+ if (ctrl_count >= ibmphp_get_total_controllers())
+ break;
+ pslot = list_entry (pslotlist, struct slot, ibm_slot_list);
+ if (pslot->ctrl->ctlr_relative_id == ctrl_count) {
+ ctrl_count++;
+ if (READ_SLOT_LATCH (pslot->ctrl)) {
+ rc = ibmphp_hpc_readslot (pslot,
+ READ_SLOTLATCHLOWREG,
+ &curlatchlow);
+ if (oldlatchlow != curlatchlow)
+ process_changeinlatch (oldlatchlow,
+ curlatchlow,
+ pslot->ctrl);
}
}
- ++poll_count;
- poll_state = POLL_SLEEP;
- break;
- case POLL_SLEEP:
- /* don't sleep with a lock on the hardware */
- up (&semOperations);
- msleep(POLL_INTERVAL_SEC * 1000);
-
- if (ibmphp_shutdown)
+ }
+ ++poll_count;
+ poll_state = POLL_SLEEP;
+ break;
+ case POLL_SLOTS:
+ list_for_each (pslotlist, &ibmphp_slot_head) {
+ pslot = list_entry (pslotlist, struct slot, ibm_slot_list);
+ // make a copy of the old status
+ memcpy ((void *) &myslot, (void *) pslot,
+ sizeof (struct slot));
+ rc = ibmphp_hpc_readslot (pslot, READ_ALLSTAT, NULL);
+ if ((myslot.status != pslot->status)
+ || (myslot.ext_status != pslot->ext_status))
+ process_changeinstatus (pslot, &myslot);
+ }
+ ctrl_count = 0x00;
+ list_for_each (pslotlist, &ibmphp_slot_head) {
+ if (ctrl_count >= ibmphp_get_total_controllers())
break;
-
- down (&semOperations);
-
- if (poll_count >= POLL_LATCH_CNT) {
- poll_count = 0;
- poll_state = POLL_SLOTS;
- } else
- poll_state = POLL_LATCH_REGISTER;
- break;
- }
- /* give up the hardware semaphore */
+ pslot = list_entry (pslotlist, struct slot, ibm_slot_list);
+ if (pslot->ctrl->ctlr_relative_id == ctrl_count) {
+ ctrl_count++;
+ if (READ_SLOT_LATCH (pslot->ctrl))
+ rc = ibmphp_hpc_readslot (pslot,
+ READ_SLOTLATCHLOWREG,
+ &curlatchlow);
+ }
+ }
+ ++poll_count;
+ poll_state = POLL_SLEEP;
+ break;
+ case POLL_SLEEP:
+ /* don't sleep with a lock on the hardware */
up (&semOperations);
- /* sleep for a short time just for good measure */
- msleep(100);
- }
- up (&sem_exit);
+ msleep(POLL_INTERVAL_SEC * 1000);
+
+ down (&semOperations);
+
+ if (poll_count >= POLL_LATCH_CNT) {
+ poll_count = 0;
+ poll_state = POLL_SLOTS;
+ } else
+ poll_state = POLL_LATCH_REGISTER;
+ break;
+ }
+ /* give up the hardware semaphore */
+ up (&semOperations);
+ /* sleep for a short time just for good measure */
+ msleep(100);
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
