cxl/region: Remove lock from memory notifier callback

In testing Dynamic Capacity Device (DCD) support, a lockdep splat
revealed an ABBA issue between the memory notifiers and the DCD extent
processing code.[0] Changing the lock ordering within DCD proved
difficult because regions must be stable while searching for the proper
region and then the device lock must be held to properly notify the DAX
region driver of memory changes.

Dan points out in the thread that notifiers should be able to trust that
it is safe to access static data. Region data is static once the device
is realized and until it's destruction. Thus it is better to manage the
notifiers within the region driver.

Remove the need for a lock by ensuring the notifiers are active only
during the region's lifetime.

Furthermore, remove cxl_region_nid() because resource can't be NULL
while the region is stable.

Link: https://lore.kernel.org/all/66b4cf539a79b_a36e829416@iweiny-mobl.notmuch/ [0]
Cc: Ying Huang <ying.huang@intel.com>
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Ying Huang <ying.huang@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Link: https://patch.msgid.link/20240904-fix-notifiers-v3-1-576b4e950266@intel.com
Signed-off-by: Dave Jiang <dave.jiang@intel.com>

authored by

Ira Weiny and committed by

Dave Jiang 2 years ago d9a476c8 3f9e0753

+30 -24

1 changed file

expand all

drivers

cxl

core

region.c

+30 -24

drivers/cxl/core/region.c

··· 2313 2313 struct cxl_region_params *p = &cxlr->params; 2314 2314 int i; 2315 2315 2316 - unregister_memory_notifier(&cxlr->memory_notifier); 2317 - unregister_mt_adistance_algorithm(&cxlr->adist_notifier); 2318 2316 device_del(&cxlr->dev); 2319 2317 2320 2318 /* ··· 2389 2391 return true; 2390 2392 } 2391 2393 2392 - static int cxl_region_nid(struct cxl_region *cxlr) 2393 - { 2394 - struct cxl_region_params *p = &cxlr->params; 2395 - struct resource *res; 2396 - 2397 - guard(rwsem_read)(&cxl_region_rwsem); 2398 - res = p->res; 2399 - if (!res) 2400 - return NUMA_NO_NODE; 2401 - return phys_to_target_node(res->start); 2402 - } 2403 - 2404 2394 static int cxl_region_perf_attrs_callback(struct notifier_block *nb, 2405 2395 unsigned long action, void *arg) 2406 2396 { ··· 2401 2415 if (nid == NUMA_NO_NODE || action != MEM_ONLINE) 2402 2416 return NOTIFY_DONE; 2403 2417 2404 - region_nid = cxl_region_nid(cxlr); 2418 + /* 2419 + * No need to hold cxl_region_rwsem; region parameters are stable 2420 + * within the cxl_region driver. 2421 + */ 2422 + region_nid = phys_to_target_node(cxlr->params.res->start); 2405 2423 if (nid != region_nid) 2406 2424 return NOTIFY_DONE; 2407 2425 ··· 2424 2434 int *adist = data; 2425 2435 int region_nid; 2426 2436 2427 - region_nid = cxl_region_nid(cxlr); 2437 + /* 2438 + * No need to hold cxl_region_rwsem; region parameters are stable 2439 + * within the cxl_region driver. 2440 + */ 2441 + region_nid = phys_to_target_node(cxlr->params.res->start); 2428 2442 if (nid != region_nid) 2429 2443 return NOTIFY_OK; 2430 2444 ··· 2477 2483 rc = device_add(dev); 2478 2484 if (rc) 2479 2485 goto err; 2480 - 2481 - cxlr->memory_notifier.notifier_call = cxl_region_perf_attrs_callback; 2482 - cxlr->memory_notifier.priority = CXL_CALLBACK_PRI; 2483 - register_memory_notifier(&cxlr->memory_notifier); 2484 - 2485 - cxlr->adist_notifier.notifier_call = cxl_region_calculate_adistance; 2486 - cxlr->adist_notifier.priority = 100; 2487 - register_mt_adistance_algorithm(&cxlr->adist_notifier); 2488 2486 2489 2487 rc = devm_add_action_or_reset(port->uport_dev, unregister_region, cxlr); 2490 2488 if (rc) ··· 3373 3387 return 1; 3374 3388 } 3375 3389 3390 + static void shutdown_notifiers(void *_cxlr) 3391 + { 3392 + struct cxl_region *cxlr = _cxlr; 3393 + 3394 + unregister_memory_notifier(&cxlr->memory_notifier); 3395 + unregister_mt_adistance_algorithm(&cxlr->adist_notifier); 3396 + } 3397 + 3376 3398 static int cxl_region_probe(struct device *dev) 3377 3399 { 3378 3400 struct cxl_region *cxlr = to_cxl_region(dev); ··· 3413 3419 out: 3414 3420 up_read(&cxl_region_rwsem); 3415 3421 3422 + if (rc) 3423 + return rc; 3424 + 3425 + cxlr->memory_notifier.notifier_call = cxl_region_perf_attrs_callback; 3426 + cxlr->memory_notifier.priority = CXL_CALLBACK_PRI; 3427 + register_memory_notifier(&cxlr->memory_notifier); 3428 + 3429 + cxlr->adist_notifier.notifier_call = cxl_region_calculate_adistance; 3430 + cxlr->adist_notifier.priority = 100; 3431 + register_mt_adistance_algorithm(&cxlr->adist_notifier); 3432 + 3433 + rc = devm_add_action_or_reset(&cxlr->dev, shutdown_notifiers, cxlr); 3416 3434 if (rc) 3417 3435 return rc; 3418 3436

Configure Feed

Configure Feed