Skip to content

Commit f8dc1bd

Browse files
djbwdavejiang
authored andcommitted
dax/hmem: Fix singleton confusion between dax_hmem_work and hmem devices
dax_hmem (ab)uses a platform device to allow for a module to autoload in the presence of "Soft Reserved" resources. The dax_hmem driver had no dependencies on the "hmem_platform" device being a singleton until the recent "dax_hmem vs dax_cxl" takeover solution. Replace the layering violation of dax_hmem_work assuming that there will never be more than one "hmem_platform" device associated with a global work item with a dax_hmem local workqueue that can theoretically support any number of hmem_platform devices. Fixup the reference counting to only pin the device while it is live in the queue. Signed-off-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Ira Weiny <ira.weiny@intel.com> Reviewed-by: Alison Schofield <alison.schofield@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Link: https://patch.msgid.link/20260327052821.440749-7-dan.j.williams@intel.com Signed-off-by: Dave Jiang <dave.jiang@intel.com>
1 parent 3cba30e commit f8dc1bd

3 files changed

Lines changed: 85 additions & 66 deletions

File tree

drivers/dax/bus.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
#ifndef __DAX_BUS_H__
44
#define __DAX_BUS_H__
55
#include <linux/device.h>
6+
#include <linux/platform_device.h>
67
#include <linux/range.h>
8+
#include <linux/workqueue.h>
79

810
struct dev_dax;
911
struct resource;
@@ -49,8 +51,19 @@ void dax_driver_unregister(struct dax_device_driver *dax_drv);
4951
void kill_dev_dax(struct dev_dax *dev_dax);
5052
bool static_dev_dax(struct dev_dax *dev_dax);
5153

54+
struct hmem_platform_device {
55+
struct platform_device pdev;
56+
struct work_struct work;
57+
bool did_probe;
58+
};
59+
60+
static inline struct hmem_platform_device *
61+
to_hmem_platform_device(struct platform_device *pdev)
62+
{
63+
return container_of(pdev, struct hmem_platform_device, pdev);
64+
}
65+
5266
#if IS_ENABLED(CONFIG_DEV_DAX_HMEM)
53-
extern bool dax_hmem_initial_probe;
5467
void dax_hmem_flush_work(void);
5568
#else
5669
static inline void dax_hmem_flush_work(void) { }

drivers/dax/hmem/device.c

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@
44
#include <linux/module.h>
55
#include <linux/dax.h>
66
#include <linux/mm.h>
7+
#include "../bus.h"
78

89
static bool nohmem;
910
module_param_named(disable, nohmem, bool, 0444);
1011

11-
bool dax_hmem_initial_probe;
12-
EXPORT_SYMBOL_FOR_MODULES(dax_hmem_initial_probe, "dax_hmem");
13-
1412
static bool platform_initialized;
1513
static DEFINE_MUTEX(hmem_resource_lock);
1614
static struct resource hmem_active = {
@@ -36,9 +34,21 @@ int walk_hmem_resources(struct device *host, walk_hmem_fn fn)
3634
}
3735
EXPORT_SYMBOL_GPL(walk_hmem_resources);
3836

37+
static void hmem_work(struct work_struct *work)
38+
{
39+
/* place holder until dax_hmem driver attaches */
40+
}
41+
42+
static struct hmem_platform_device hmem_platform = {
43+
.pdev = {
44+
.name = "hmem_platform",
45+
.id = 0,
46+
},
47+
.work = __WORK_INITIALIZER(hmem_platform.work, hmem_work),
48+
};
49+
3950
static void __hmem_register_resource(int target_nid, struct resource *res)
4051
{
41-
struct platform_device *pdev;
4252
struct resource *new;
4353
int rc;
4454

@@ -54,17 +64,13 @@ static void __hmem_register_resource(int target_nid, struct resource *res)
5464
if (platform_initialized)
5565
return;
5666

57-
pdev = platform_device_alloc("hmem_platform", 0);
58-
if (!pdev) {
67+
rc = platform_device_register(&hmem_platform.pdev);
68+
if (rc) {
5969
pr_err_once("failed to register device-dax hmem_platform device\n");
6070
return;
6171
}
6272

63-
rc = platform_device_add(pdev);
64-
if (rc)
65-
platform_device_put(pdev);
66-
else
67-
platform_initialized = true;
73+
platform_initialized = true;
6874
}
6975

7076
void hmem_register_resource(int target_nid, struct resource *res)

drivers/dax/hmem/hmem.c

Lines changed: 54 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -59,20 +59,11 @@ static void release_hmem(void *pdev)
5959
platform_device_unregister(pdev);
6060
}
6161

62-
struct dax_defer_work {
63-
struct platform_device *pdev;
64-
struct work_struct work;
65-
};
66-
67-
static void process_defer_work(struct work_struct *w);
68-
69-
static struct dax_defer_work dax_hmem_work = {
70-
.work = __WORK_INITIALIZER(dax_hmem_work.work, process_defer_work),
71-
};
62+
static struct workqueue_struct *dax_hmem_wq;
7263

7364
void dax_hmem_flush_work(void)
7465
{
75-
flush_work(&dax_hmem_work.work);
66+
flush_workqueue(dax_hmem_wq);
7667
}
7768
EXPORT_SYMBOL_FOR_MODULES(dax_hmem_flush_work, "dax_cxl");
7869

@@ -134,24 +125,6 @@ static int __hmem_register_device(struct device *host, int target_nid,
134125
return rc;
135126
}
136127

137-
static int hmem_register_device(struct device *host, int target_nid,
138-
const struct resource *res)
139-
{
140-
if (IS_ENABLED(CONFIG_DEV_DAX_CXL) &&
141-
region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
142-
IORES_DESC_CXL) != REGION_DISJOINT) {
143-
if (!dax_hmem_initial_probe) {
144-
dev_dbg(host, "await CXL initial probe: %pr\n", res);
145-
queue_work(system_long_wq, &dax_hmem_work.work);
146-
return 0;
147-
}
148-
dev_dbg(host, "deferring range to CXL: %pr\n", res);
149-
return 0;
150-
}
151-
152-
return __hmem_register_device(host, target_nid, res);
153-
}
154-
155128
static int hmem_register_cxl_device(struct device *host, int target_nid,
156129
const struct resource *res)
157130
{
@@ -170,35 +143,55 @@ static int hmem_register_cxl_device(struct device *host, int target_nid,
170143

171144
static void process_defer_work(struct work_struct *w)
172145
{
173-
struct dax_defer_work *work = container_of(w, typeof(*work), work);
174-
struct platform_device *pdev;
175-
176-
if (!work->pdev)
177-
return;
178-
179-
pdev = work->pdev;
146+
struct hmem_platform_device *hpdev = container_of(w, typeof(*hpdev), work);
147+
struct device *dev = &hpdev->pdev.dev;
180148

181149
/* Relies on cxl_acpi and cxl_pci having had a chance to load */
182150
wait_for_device_probe();
183151

184-
guard(device)(&pdev->dev);
185-
if (!pdev->dev.driver)
186-
return;
152+
guard(device)(dev);
153+
if (!dev->driver)
154+
goto out;
187155

188-
if (!dax_hmem_initial_probe) {
189-
dax_hmem_initial_probe = true;
190-
walk_hmem_resources(&pdev->dev, hmem_register_cxl_device);
156+
if (!hpdev->did_probe) {
157+
hpdev->did_probe = true;
158+
walk_hmem_resources(dev, hmem_register_cxl_device);
191159
}
160+
out:
161+
put_device(dev);
162+
}
163+
164+
static int hmem_register_device(struct device *host, int target_nid,
165+
const struct resource *res)
166+
{
167+
struct platform_device *pdev = to_platform_device(host);
168+
struct hmem_platform_device *hpdev = to_hmem_platform_device(pdev);
169+
170+
if (IS_ENABLED(CONFIG_DEV_DAX_CXL) &&
171+
region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
172+
IORES_DESC_CXL) != REGION_DISJOINT) {
173+
if (!hpdev->did_probe) {
174+
dev_dbg(host, "await CXL initial probe: %pr\n", res);
175+
hpdev->work.func = process_defer_work;
176+
get_device(host);
177+
if (!queue_work(dax_hmem_wq, &hpdev->work))
178+
put_device(host);
179+
return 0;
180+
}
181+
dev_dbg(host, "deferring range to CXL: %pr\n", res);
182+
return 0;
183+
}
184+
185+
return __hmem_register_device(host, target_nid, res);
192186
}
193187

194188
static int dax_hmem_platform_probe(struct platform_device *pdev)
195189
{
196-
if (work_pending(&dax_hmem_work.work))
197-
return -EBUSY;
190+
struct hmem_platform_device *hpdev = to_hmem_platform_device(pdev);
198191

199-
if (!dax_hmem_work.pdev)
200-
dax_hmem_work.pdev =
201-
to_platform_device(get_device(&pdev->dev));
192+
/* queue is only flushed on module unload, fail rebind with pending work */
193+
if (work_pending(&hpdev->work))
194+
return -EBUSY;
202195

203196
return walk_hmem_resources(&pdev->dev, hmem_register_device);
204197
}
@@ -224,26 +217,33 @@ static __init int dax_hmem_init(void)
224217
request_module("cxl_pci");
225218
}
226219

220+
dax_hmem_wq = alloc_ordered_workqueue("dax_hmem_wq", 0);
221+
if (!dax_hmem_wq)
222+
return -ENOMEM;
223+
227224
rc = platform_driver_register(&dax_hmem_platform_driver);
228225
if (rc)
229-
return rc;
226+
goto err_platform_driver;
230227

231228
rc = platform_driver_register(&dax_hmem_driver);
232229
if (rc)
233-
platform_driver_unregister(&dax_hmem_platform_driver);
230+
goto err_driver;
231+
232+
return 0;
233+
234+
err_driver:
235+
platform_driver_unregister(&dax_hmem_platform_driver);
236+
err_platform_driver:
237+
destroy_workqueue(dax_hmem_wq);
234238

235239
return rc;
236240
}
237241

238242
static __exit void dax_hmem_exit(void)
239243
{
240-
if (dax_hmem_work.pdev) {
241-
flush_work(&dax_hmem_work.work);
242-
put_device(&dax_hmem_work.pdev->dev);
243-
}
244-
245244
platform_driver_unregister(&dax_hmem_driver);
246245
platform_driver_unregister(&dax_hmem_platform_driver);
246+
destroy_workqueue(dax_hmem_wq);
247247
}
248248

249249
module_init(dax_hmem_init);

0 commit comments

Comments
 (0)