Messages in this thread | | | From | "" <> | Subject | Re: [PATCH] nvdimm: nvdimm_bus_register: Avoid adding device to the unregistered bus | Date | Tue, 21 Mar 2023 02:17:36 +0000 |
| |
On 21/03/2023 01:30, Dan Williams wrote: > lizhijian@fujitsu.com wrote: > [..] >>>> Dan, >>>> >>>> Configure the kconfig with ACPI_NFIT [=m] && LIBNVDIMM [=y], and add extra kernel booting parameter >>>> 'initcall_blacklist=libnvdimm_init'. Then kernel panic! >>> >>> That's expected though, >> >> Do you mean we just keep it as it is. > > Yes. > >> >> >>> you can't block libnvdimm_init and then expect >>> modules that link to libnvdimm to work. >> Ah, we would rather see it *unable to work* than panic, isn't it. > > That part is true, but consider the implications of adding error > handling to all code that can no longer depend on initcall ordering, not > just libnvdimm. This would be a large paradigm shift. > > Now I do think it would be a good idea to fail device_add() if the bus > is not registered, but I *think* that happens now as a result of: > > 5221b82d46f2 driver core: bus: bus_add/probe/remove_device() cleanups > > ...can you double check if you have that commit in your tests?
Great, panic is gone after i upgraded to the upstream!
> Now I do think it would be a good idea to fail device_add() if the bus > is not registered,
BTW, below line 369: device_add() didn't fail in practical.
354 mutex_init(&nvdimm_bus->reconfig_mutex); 355 badrange_init(&nvdimm_bus->badrange); 356 nvdimm_bus->nd_desc = nd_desc; 357 nvdimm_bus->dev.parent = parent; 358 nvdimm_bus->dev.type = &nvdimm_bus_dev_type; 359 nvdimm_bus->dev.groups = nd_desc->attr_groups; 360 nvdimm_bus->dev.bus = &nvdimm_bus_type; 361 nvdimm_bus->dev.of_node = nd_desc->of_node; 362 device_initialize(&nvdimm_bus->dev); 363 lockdep_set_class(&nvdimm_bus->dev.mutex, &nvdimm_bus_key); 364 device_set_pm_not_required(&nvdimm_bus->dev); 365 rc = dev_set_name(&nvdimm_bus->dev, "ndbus%d", nvdimm_bus->id); 366 if (rc) 367 goto err; 368 369 rc = device_add(&nvdimm_bus->dev); 370 dev_err(&nvdimm_bus->dev, "registration failed: %d\n", rc);
Thanks Zhijian
| |