Skip to content

Commit a98d1a0

Browse files
committed
scsi: qedi: Convert to hotplug state machine
The CPU hotplug code is a trainwreck. It leaks a notifier in case of driver registration error and the per cpu loop is racy against cpu hotplug. Aside of that the driver should have been written and merged with the new state machine interfaces in the first place. Mop up the mess and Convert it to the hotplug state machine. Signed-off-by: Thomas Grumpy Gleixner <[email protected]> Cc: Nilesh Javali <[email protected]> Cc: Adheer Chandravanshi <[email protected]> Cc: Chad Dupuis <[email protected]> Cc: Saurav Kashyap <[email protected]> Cc: Arun Easi <[email protected]> Cc: Manish Rangankar <[email protected]> Cc: Johannes Thumshirn <[email protected]> Cc: Hannes Reinecke <[email protected]> Cc: Martin K. Petersen <[email protected]> Cc: James Bottomley <[email protected]>
1 parent 6ac3bb1 commit a98d1a0

File tree

1 file changed

+32
-64
lines changed

1 file changed

+32
-64
lines changed

drivers/scsi/qedi/qedi_main.c

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1612,30 +1612,29 @@ static int qedi_percpu_io_thread(void *arg)
16121612
return 0;
16131613
}
16141614

1615-
static void qedi_percpu_thread_create(unsigned int cpu)
1615+
static int qedi_cpu_online(unsigned int cpu)
16161616
{
1617-
struct qedi_percpu_s *p;
1617+
struct qedi_percpu_s *p = this_cpu_ptr(&qedi_percpu);
16181618
struct task_struct *thread;
16191619

1620-
p = &per_cpu(qedi_percpu, cpu);
1621-
16221620
thread = kthread_create_on_node(qedi_percpu_io_thread, (void *)p,
16231621
cpu_to_node(cpu),
16241622
"qedi_thread/%d", cpu);
1625-
if (likely(!IS_ERR(thread))) {
1626-
kthread_bind(thread, cpu);
1627-
p->iothread = thread;
1628-
wake_up_process(thread);
1629-
}
1623+
if (IS_ERR(thread))
1624+
return PTR_ERR(thread);
1625+
1626+
kthread_bind(thread, cpu);
1627+
p->iothread = thread;
1628+
wake_up_process(thread);
1629+
return 0;
16301630
}
16311631

1632-
static void qedi_percpu_thread_destroy(unsigned int cpu)
1632+
static int qedi_cpu_offline(unsigned int cpu)
16331633
{
1634-
struct qedi_percpu_s *p;
1635-
struct task_struct *thread;
1634+
struct qedi_percpu_s *p = this_cpu_ptr(&qedi_percpu);
16361635
struct qedi_work *work, *tmp;
1636+
struct task_struct *thread;
16371637

1638-
p = &per_cpu(qedi_percpu, cpu);
16391638
spin_lock_bh(&p->p_work_lock);
16401639
thread = p->iothread;
16411640
p->iothread = NULL;
@@ -1650,35 +1649,9 @@ static void qedi_percpu_thread_destroy(unsigned int cpu)
16501649
spin_unlock_bh(&p->p_work_lock);
16511650
if (thread)
16521651
kthread_stop(thread);
1652+
return 0;
16531653
}
16541654

1655-
static int qedi_cpu_callback(struct notifier_block *nfb,
1656-
unsigned long action, void *hcpu)
1657-
{
1658-
unsigned int cpu = (unsigned long)hcpu;
1659-
1660-
switch (action) {
1661-
case CPU_ONLINE:
1662-
case CPU_ONLINE_FROZEN:
1663-
QEDI_ERR(NULL, "CPU %d online.\n", cpu);
1664-
qedi_percpu_thread_create(cpu);
1665-
break;
1666-
case CPU_DEAD:
1667-
case CPU_DEAD_FROZEN:
1668-
QEDI_ERR(NULL, "CPU %d offline.\n", cpu);
1669-
qedi_percpu_thread_destroy(cpu);
1670-
break;
1671-
default:
1672-
break;
1673-
}
1674-
1675-
return NOTIFY_OK;
1676-
}
1677-
1678-
static struct notifier_block qedi_cpu_notifier = {
1679-
.notifier_call = qedi_cpu_callback,
1680-
};
1681-
16821655
void qedi_reset_host_mtu(struct qedi_ctx *qedi, u16 mtu)
16831656
{
16841657
struct qed_ll2_params params;
@@ -2038,6 +2011,8 @@ static struct pci_device_id qedi_pci_tbl[] = {
20382011
};
20392012
MODULE_DEVICE_TABLE(pci, qedi_pci_tbl);
20402013

2014+
static enum cpuhp_state qedi_cpuhp_state;
2015+
20412016
static struct pci_driver qedi_pci_driver = {
20422017
.name = QEDI_MODULE_NAME,
20432018
.id_table = qedi_pci_tbl,
@@ -2047,16 +2022,13 @@ static struct pci_driver qedi_pci_driver = {
20472022

20482023
static int __init qedi_init(void)
20492024
{
2050-
int rc = 0;
2051-
int ret;
20522025
struct qedi_percpu_s *p;
2053-
unsigned int cpu = 0;
2026+
int cpu, rc = 0;
20542027

20552028
qedi_ops = qed_get_iscsi_ops();
20562029
if (!qedi_ops) {
20572030
QEDI_ERR(NULL, "Failed to get qed iSCSI operations\n");
2058-
rc = -EINVAL;
2059-
goto exit_qedi_init_0;
2031+
return -EINVAL;
20602032
}
20612033

20622034
#ifdef CONFIG_DEBUG_FS
@@ -2070,47 +2042,43 @@ static int __init qedi_init(void)
20702042
goto exit_qedi_init_1;
20712043
}
20722044

2073-
register_hotcpu_notifier(&qedi_cpu_notifier);
2074-
2075-
ret = pci_register_driver(&qedi_pci_driver);
2076-
if (ret) {
2077-
QEDI_ERR(NULL, "Failed to register driver\n");
2078-
rc = -EINVAL;
2079-
goto exit_qedi_init_2;
2080-
}
2081-
20822045
for_each_possible_cpu(cpu) {
20832046
p = &per_cpu(qedi_percpu, cpu);
20842047
INIT_LIST_HEAD(&p->work_list);
20852048
spin_lock_init(&p->p_work_lock);
20862049
p->iothread = NULL;
20872050
}
20882051

2089-
for_each_online_cpu(cpu)
2090-
qedi_percpu_thread_create(cpu);
2052+
rc = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "scsi/qedi:online",
2053+
qedi_cpu_online, qedi_cpu_offline);
2054+
if (rc < 0)
2055+
goto exit_qedi_init_2;
2056+
qedi_cpuhp_state = rc;
20912057

2092-
return rc;
2058+
rc = pci_register_driver(&qedi_pci_driver);
2059+
if (rc) {
2060+
QEDI_ERR(NULL, "Failed to register driver\n");
2061+
goto exit_qedi_hp;
2062+
}
2063+
2064+
return 0;
20932065

2066+
exit_qedi_hp:
2067+
cpuhp_remove_state(qedi_cpuhp_state);
20942068
exit_qedi_init_2:
20952069
iscsi_unregister_transport(&qedi_iscsi_transport);
20962070
exit_qedi_init_1:
20972071
#ifdef CONFIG_DEBUG_FS
20982072
qedi_dbg_exit();
20992073
#endif
21002074
qed_put_iscsi_ops();
2101-
exit_qedi_init_0:
21022075
return rc;
21032076
}
21042077

21052078
static void __exit qedi_cleanup(void)
21062079
{
2107-
unsigned int cpu = 0;
2108-
2109-
for_each_online_cpu(cpu)
2110-
qedi_percpu_thread_destroy(cpu);
2111-
21122080
pci_unregister_driver(&qedi_pci_driver);
2113-
unregister_hotcpu_notifier(&qedi_cpu_notifier);
2081+
cpuhp_remove_state(qedi_cpuhp_state);
21142082
iscsi_unregister_transport(&qedi_iscsi_transport);
21152083

21162084
#ifdef CONFIG_DEBUG_FS

0 commit comments

Comments
 (0)