Skip to content

Commit 02722a8

Browse files
committed
Merge branch 'remotes/lorenzo/pci/hv'
- Fix race when removing device (Long Li) - Remove unused bus device removal refcount/functions (Long Li) * remotes/lorenzo/pci/hv: PCI: hv: Remove bus device removal unused refcount/functions PCI: hv: Fix a race condition when removing the device
2 parents 777e5e6 + 326dc2e commit 02722a8

1 file changed

Lines changed: 26 additions & 38 deletions

File tree

drivers/pci/controller/pci-hyperv.c

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,6 @@ enum hv_pcibus_state {
444444
hv_pcibus_probed,
445445
hv_pcibus_installed,
446446
hv_pcibus_removing,
447-
hv_pcibus_removed,
448447
hv_pcibus_maximum
449448
};
450449

@@ -453,15 +452,13 @@ struct hv_pcibus_device {
453452
/* Protocol version negotiated with the host */
454453
enum pci_protocol_version_t protocol_version;
455454
enum hv_pcibus_state state;
456-
refcount_t remove_lock;
457455
struct hv_device *hdev;
458456
resource_size_t low_mmio_space;
459457
resource_size_t high_mmio_space;
460458
struct resource *mem_config;
461459
struct resource *low_mmio_res;
462460
struct resource *high_mmio_res;
463461
struct completion *survey_event;
464-
struct completion remove_event;
465462
struct pci_bus *pci_bus;
466463
spinlock_t config_lock; /* Avoid two threads writing index page */
467464
spinlock_t device_list_lock; /* Protect lists below */
@@ -593,9 +590,6 @@ static void put_pcichild(struct hv_pci_dev *hpdev)
593590
kfree(hpdev);
594591
}
595592

596-
static void get_hvpcibus(struct hv_pcibus_device *hv_pcibus);
597-
static void put_hvpcibus(struct hv_pcibus_device *hv_pcibus);
598-
599593
/*
600594
* There is no good way to get notified from vmbus_onoffer_rescind(),
601595
* so let's use polling here, since this is not a hot path.
@@ -2064,10 +2058,8 @@ static void pci_devices_present_work(struct work_struct *work)
20642058
}
20652059
spin_unlock_irqrestore(&hbus->device_list_lock, flags);
20662060

2067-
if (!dr) {
2068-
put_hvpcibus(hbus);
2061+
if (!dr)
20692062
return;
2070-
}
20712063

20722064
/* First, mark all existing children as reported missing. */
20732065
spin_lock_irqsave(&hbus->device_list_lock, flags);
@@ -2150,7 +2142,6 @@ static void pci_devices_present_work(struct work_struct *work)
21502142
break;
21512143
}
21522144

2153-
put_hvpcibus(hbus);
21542145
kfree(dr);
21552146
}
21562147

@@ -2191,12 +2182,10 @@ static int hv_pci_start_relations_work(struct hv_pcibus_device *hbus,
21912182
list_add_tail(&dr->list_entry, &hbus->dr_list);
21922183
spin_unlock_irqrestore(&hbus->device_list_lock, flags);
21932184

2194-
if (pending_dr) {
2185+
if (pending_dr)
21952186
kfree(dr_wrk);
2196-
} else {
2197-
get_hvpcibus(hbus);
2187+
else
21982188
queue_work(hbus->wq, &dr_wrk->wrk);
2199-
}
22002189

22012190
return 0;
22022191
}
@@ -2339,8 +2328,6 @@ static void hv_eject_device_work(struct work_struct *work)
23392328
put_pcichild(hpdev);
23402329
put_pcichild(hpdev);
23412330
/* hpdev has been freed. Do not use it any more. */
2342-
2343-
put_hvpcibus(hbus);
23442331
}
23452332

23462333
/**
@@ -2364,7 +2351,6 @@ static void hv_pci_eject_device(struct hv_pci_dev *hpdev)
23642351
hpdev->state = hv_pcichild_ejecting;
23652352
get_pcichild(hpdev);
23662353
INIT_WORK(&hpdev->wrk, hv_eject_device_work);
2367-
get_hvpcibus(hbus);
23682354
queue_work(hbus->wq, &hpdev->wrk);
23692355
}
23702356

@@ -2964,17 +2950,6 @@ static int hv_send_resources_released(struct hv_device *hdev)
29642950
return 0;
29652951
}
29662952

2967-
static void get_hvpcibus(struct hv_pcibus_device *hbus)
2968-
{
2969-
refcount_inc(&hbus->remove_lock);
2970-
}
2971-
2972-
static void put_hvpcibus(struct hv_pcibus_device *hbus)
2973-
{
2974-
if (refcount_dec_and_test(&hbus->remove_lock))
2975-
complete(&hbus->remove_event);
2976-
}
2977-
29782953
#define HVPCI_DOM_MAP_SIZE (64 * 1024)
29792954
static DECLARE_BITMAP(hvpci_dom_map, HVPCI_DOM_MAP_SIZE);
29802955

@@ -3094,14 +3069,12 @@ static int hv_pci_probe(struct hv_device *hdev,
30943069
hbus->sysdata.domain = dom;
30953070

30963071
hbus->hdev = hdev;
3097-
refcount_set(&hbus->remove_lock, 1);
30983072
INIT_LIST_HEAD(&hbus->children);
30993073
INIT_LIST_HEAD(&hbus->dr_list);
31003074
INIT_LIST_HEAD(&hbus->resources_for_children);
31013075
spin_lock_init(&hbus->config_lock);
31023076
spin_lock_init(&hbus->device_list_lock);
31033077
spin_lock_init(&hbus->retarget_msi_interrupt_lock);
3104-
init_completion(&hbus->remove_event);
31053078
hbus->wq = alloc_ordered_workqueue("hv_pci_%x", 0,
31063079
hbus->sysdata.domain);
31073080
if (!hbus->wq) {
@@ -3243,8 +3216,9 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
32433216
struct pci_packet teardown_packet;
32443217
u8 buffer[sizeof(struct pci_message)];
32453218
} pkt;
3246-
struct hv_dr_state *dr;
32473219
struct hv_pci_compl comp_pkt;
3220+
struct hv_pci_dev *hpdev, *tmp;
3221+
unsigned long flags;
32483222
int ret;
32493223

32503224
/*
@@ -3256,9 +3230,16 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
32563230

32573231
if (!keep_devs) {
32583232
/* Delete any children which might still exist. */
3259-
dr = kzalloc(sizeof(*dr), GFP_KERNEL);
3260-
if (dr && hv_pci_start_relations_work(hbus, dr))
3261-
kfree(dr);
3233+
spin_lock_irqsave(&hbus->device_list_lock, flags);
3234+
list_for_each_entry_safe(hpdev, tmp, &hbus->children, list_entry) {
3235+
list_del(&hpdev->list_entry);
3236+
if (hpdev->pci_slot)
3237+
pci_destroy_slot(hpdev->pci_slot);
3238+
/* For the two refs got in new_pcichild_device() */
3239+
put_pcichild(hpdev);
3240+
put_pcichild(hpdev);
3241+
}
3242+
spin_unlock_irqrestore(&hbus->device_list_lock, flags);
32623243
}
32633244

32643245
ret = hv_send_resources_released(hdev);
@@ -3301,13 +3282,23 @@ static int hv_pci_remove(struct hv_device *hdev)
33013282

33023283
hbus = hv_get_drvdata(hdev);
33033284
if (hbus->state == hv_pcibus_installed) {
3285+
tasklet_disable(&hdev->channel->callback_event);
3286+
hbus->state = hv_pcibus_removing;
3287+
tasklet_enable(&hdev->channel->callback_event);
3288+
destroy_workqueue(hbus->wq);
3289+
hbus->wq = NULL;
3290+
/*
3291+
* At this point, no work is running or can be scheduled
3292+
* on hbus-wq. We can't race with hv_pci_devices_present()
3293+
* or hv_pci_eject_device(), it's safe to proceed.
3294+
*/
3295+
33043296
/* Remove the bus from PCI's point of view. */
33053297
pci_lock_rescan_remove();
33063298
pci_stop_root_bus(hbus->pci_bus);
33073299
hv_pci_remove_slots(hbus);
33083300
pci_remove_root_bus(hbus->pci_bus);
33093301
pci_unlock_rescan_remove();
3310-
hbus->state = hv_pcibus_removed;
33113302
}
33123303

33133304
ret = hv_pci_bus_exit(hdev, false);
@@ -3320,9 +3311,6 @@ static int hv_pci_remove(struct hv_device *hdev)
33203311
hv_pci_free_bridge_windows(hbus);
33213312
irq_domain_remove(hbus->irq_domain);
33223313
irq_domain_free_fwnode(hbus->sysdata.fwnode);
3323-
put_hvpcibus(hbus);
3324-
wait_for_completion(&hbus->remove_event);
3325-
destroy_workqueue(hbus->wq);
33263314

33273315
hv_put_dom_num(hbus->sysdata.domain);
33283316

0 commit comments

Comments
 (0)