On 06/06/2025 00:54, Dave Jiang wrote:
>> -static int hmem_register_device(struct device *host, int target_nid,
>> - const struct resource *res)
>> +static int hmem_register_device(int target_nid, const struct resource *res)
>> {
>> + struct device *host = &dax_hmem_pdev->dev;
>> struct platform_device *pdev;
>> struct memregion_info info;
>> long id;
>> @@ -125,7 +127,8 @@ static int hmem_register_device(struct device *host, int
>> target_nid,
>>
>> static int dax_hmem_platform_probe(struct platform_device *pdev)
>> {
>> - return walk_hmem_resources(&pdev->dev, hmem_register_device);
>> + dax_hmem_pdev = pdev;
> Is there never more than 1 DAX HMEM platform device that can show up? The
> global pointer makes me nervous.
IIUC,
Referring to the device creation logic in `__hmem_register_resource()` (shown
below),
only one `hmem_platform` instance can ever be registered. This ensures the
change is safe.
However, I agree that using a global pointer in a function that may be called
multiple times
does raise valid concerns.
To strengthen this, how about:
1. Add a comment clarifying single-instance enforcement
2. Add a warn_on/bug_on for it: `WARN_ON(dax_hmem_pdev && dax_hmem_pdev !=
pdev)`
static void __hmem_register_resource(int target_nid, struct resource *res)
{
struct platform_device *pdev;
struct resource *new;
int rc;
new = __request_region(&hmem_active, res->start, resource_size(res), "",
0);
if (!new) {
pr_debug("hmem range %pr already active\n", res);
return;
}
new->desc = target_nid;
if (platform_initialized)
return;
pdev = platform_device_alloc("hmem_platform", 0);
if (!pdev) {
pr_err_once("failed to register device-dax hmem_platform
device\n");
return;
}
rc = platform_device_add(pdev);
if (rc)
platform_device_put(pdev);
else
platform_initialized = true;
}
Thanks
Zhijian
>
> DJ
