On 21/03/2023 01:30, Dan Williams wrote: > lizhij...@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
