From kristen.c.accardi@intel.com Fri Sep 22 10:24:28 2006 Date: Fri, 22 Sep 2006 10:17:29 -0700 From: Kristen Carlson Accardi To: greg@kroah.com, Kristen Accardi Cc: Kenji Kaneshige Subject: pciehp - add missing locking Message-Id: <20060922101729.fd87c0ef.kristen.c.accardi@intel.com> From: Kenji Kaneshige This patch fixes the problem that system will panic if multiple power on/off operations are issued to the same slot in parallel. This problem can be easily reproduced by commands below. # while true; do echo 1 > power; echo 0 > power; done & # while true; do echo 1 > power; echo 0 > power; done & The cause is lack of locking for enable/disable operations. This patch fixes this problem. Signed-off-by: Kenji Kaneshige Signed-off-by: Kristen Carlson Accardi Signed-off-by: Greg Kroah-Hartman --- drivers/pci/hotplug/pciehp.h | 1 drivers/pci/hotplug/pciehp_core.c | 6 ++-- drivers/pci/hotplug/pciehp_ctrl.c | 54 +++++++++++++++++++------------------- drivers/pci/hotplug/pciehp_hpc.c | 2 + 4 files changed, 33 insertions(+), 30 deletions(-) --- gregkh-2.6.orig/drivers/pci/hotplug/pciehp.h +++ gregkh-2.6/drivers/pci/hotplug/pciehp.h @@ -92,6 +92,7 @@ struct php_ctlr_state_s { struct controller { struct controller *next; struct mutex crit_sect; /* critical section mutex */ + struct mutex ctrl_lock; /* controller lock */ struct php_ctlr_state_s *hpc_ctlr_handle; /* HPC controller handle */ int num_slots; /* Number of slots on ctlr */ int slot_num_inc; /* 1 or -1 */ --- gregkh-2.6.orig/drivers/pci/hotplug/pciehp_core.c +++ gregkh-2.6/drivers/pci/hotplug/pciehp_core.c @@ -448,7 +448,7 @@ static int pciehp_probe(struct pcie_devi } /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); t_slot->hpc_ops->get_adapter_status(t_slot, &value); /* Check if slot is occupied */ @@ -456,7 +456,7 @@ static int pciehp_probe(struct pcie_devi rc = t_slot->hpc_ops->power_off_slot(t_slot); /* Power off slot if not occupied*/ if (rc) { /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); goto err_out_free_ctrl_slot; } else /* Wait for the command to complete */ @@ -464,7 +464,7 @@ static int pciehp_probe(struct pcie_devi } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return 0; --- gregkh-2.6.orig/drivers/pci/hotplug/pciehp_ctrl.c +++ gregkh-2.6/drivers/pci/hotplug/pciehp_ctrl.c @@ -234,13 +234,13 @@ u8 pciehp_handle_power_fault(u8 hp_slot, static void set_slot_off(struct controller *ctrl, struct slot * pslot) { /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); /* turn off slot, turn on Amber LED, turn off Green LED if supported*/ if (POWER_CTRL(ctrl->ctrlcap)) { if (pslot->hpc_ops->power_off_slot(pslot)) { err("%s: Issue of Slot Power Off command failed\n", __FUNCTION__); - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return; } wait_for_ctrl_irq (ctrl); @@ -254,14 +254,14 @@ static void set_slot_off(struct controll if (ATTN_LED(ctrl->ctrlcap)) { if (pslot->hpc_ops->set_attention_status(pslot, 1)) { err("%s: Issue of Set Attention Led command failed\n", __FUNCTION__); - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return; } wait_for_ctrl_irq (ctrl); } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); } /** @@ -284,13 +284,13 @@ static int board_added(struct slot *p_sl ctrl->slot_device_offset, hp_slot); /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (POWER_CTRL(ctrl->ctrlcap)) { /* Power on slot */ rc = p_slot->hpc_ops->power_on_slot(p_slot); if (rc) { - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return -1; } @@ -306,7 +306,7 @@ static int board_added(struct slot *p_sl } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); /* Wait for ~1 second */ wait_for_ctrl_irq (ctrl); @@ -340,7 +340,7 @@ static int board_added(struct slot *p_sl pci_fixup_device(pci_fixup_final, ctrl->pci_dev); if (PWR_LED(ctrl->ctrlcap)) { /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); p_slot->hpc_ops->green_led_on(p_slot); @@ -348,7 +348,7 @@ static int board_added(struct slot *p_sl wait_for_ctrl_irq (ctrl); /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); } return 0; @@ -380,14 +380,14 @@ static int remove_board(struct slot *p_s dbg("In %s, hp_slot = %d\n", __FUNCTION__, hp_slot); /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (POWER_CTRL(ctrl->ctrlcap)) { /* power off slot */ rc = p_slot->hpc_ops->power_off_slot(p_slot); if (rc) { err("%s: Issue of Slot Disable command failed\n", __FUNCTION__); - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return rc; } /* Wait for the command to complete */ @@ -403,7 +403,7 @@ static int remove_board(struct slot *p_s } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); return 0; } @@ -450,7 +450,7 @@ static void pciehp_pushbutton_thread(uns if (pciehp_enable_slot(p_slot) && PWR_LED(p_slot->ctrl->ctrlcap)) { /* Wait for exclusive access to hardware */ - mutex_lock(&p_slot->ctrl->crit_sect); + mutex_lock(&p_slot->ctrl->ctrl_lock); p_slot->hpc_ops->green_led_off(p_slot); @@ -458,7 +458,7 @@ static void pciehp_pushbutton_thread(uns wait_for_ctrl_irq (p_slot->ctrl); /* Done with exclusive hardware access */ - mutex_unlock(&p_slot->ctrl->crit_sect); + mutex_unlock(&p_slot->ctrl->ctrl_lock); } p_slot->state = STATIC_STATE; } @@ -500,7 +500,7 @@ static void pciehp_surprise_rm_thread(un if (pciehp_enable_slot(p_slot) && PWR_LED(p_slot->ctrl->ctrlcap)) { /* Wait for exclusive access to hardware */ - mutex_lock(&p_slot->ctrl->crit_sect); + mutex_lock(&p_slot->ctrl->ctrl_lock); p_slot->hpc_ops->green_led_off(p_slot); @@ -508,7 +508,7 @@ static void pciehp_surprise_rm_thread(un wait_for_ctrl_irq (p_slot->ctrl); /* Done with exclusive hardware access */ - mutex_unlock(&p_slot->ctrl->crit_sect); + mutex_unlock(&p_slot->ctrl->ctrl_lock); } p_slot->state = STATIC_STATE; } @@ -621,7 +621,7 @@ static void interrupt_event_handler(stru switch (p_slot->state) { case BLINKINGOFF_STATE: /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (PWR_LED(ctrl->ctrlcap)) { p_slot->hpc_ops->green_led_on(p_slot); @@ -635,11 +635,11 @@ static void interrupt_event_handler(stru wait_for_ctrl_irq (ctrl); } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); break; case BLINKINGON_STATE: /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (PWR_LED(ctrl->ctrlcap)) { p_slot->hpc_ops->green_led_off(p_slot); @@ -652,7 +652,7 @@ static void interrupt_event_handler(stru wait_for_ctrl_irq (ctrl); } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); break; default: @@ -681,7 +681,7 @@ static void interrupt_event_handler(stru } /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); /* blink green LED and turn off amber */ if (PWR_LED(ctrl->ctrlcap)) { @@ -698,7 +698,7 @@ static void interrupt_event_handler(stru } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); init_timer(&p_slot->task_event); p_slot->task_event.expires = jiffies + 5 * HZ; /* 5 second delay */ @@ -713,7 +713,7 @@ static void interrupt_event_handler(stru if (POWER_CTRL(ctrl->ctrlcap)) { dbg("power fault\n"); /* Wait for exclusive access to hardware */ - mutex_lock(&ctrl->crit_sect); + mutex_lock(&ctrl->ctrl_lock); if (ATTN_LED(ctrl->ctrlcap)) { p_slot->hpc_ops->set_attention_status(p_slot, 1); @@ -726,7 +726,7 @@ static void interrupt_event_handler(stru } /* Done with exclusive hardware access */ - mutex_unlock(&ctrl->crit_sect); + mutex_unlock(&ctrl->ctrl_lock); } } /***********SURPRISE REMOVAL********************/ @@ -789,7 +789,6 @@ int pciehp_enable_slot(struct slot *p_sl return -EINVAL; } } - mutex_unlock(&p_slot->ctrl->crit_sect); p_slot->hpc_ops->get_latch_status(p_slot, &getstatus); @@ -801,6 +800,7 @@ int pciehp_enable_slot(struct slot *p_sl if (p_slot) update_slot_info(p_slot); + mutex_unlock(&p_slot->ctrl->crit_sect); return rc; } @@ -846,10 +846,10 @@ int pciehp_disable_slot(struct slot *p_s } } - mutex_unlock(&p_slot->ctrl->crit_sect); - ret = remove_board(p_slot); update_slot_info(p_slot); + + mutex_unlock(&p_slot->ctrl->crit_sect); return ret; } --- gregkh-2.6.orig/drivers/pci/hotplug/pciehp_hpc.c +++ gregkh-2.6/drivers/pci/hotplug/pciehp_hpc.c @@ -1402,6 +1402,8 @@ int pcie_init(struct controller * ctrl, pdev->subsystem_vendor, pdev->subsystem_device); mutex_init(&ctrl->crit_sect); + mutex_init(&ctrl->ctrl_lock); + /* setup wait queue */ init_waitqueue_head(&ctrl->queue);