On 2025/4/12 11:18, liwei (JK) wrote:
在 2025/4/10 19:19, Su Hui 写道:
On 2025/4/10 11:28, Xiangwei Li wrote:
This reverts commit 804443c1f27883926de94c849d91f5b7d7d696e9.
The newly added logic incorrectly sets bus_registered to true even when
device_register returns an error, this is incorrect.
When device_register fails, there is no need to release the
reference count,
I think you missed some thing about device_register(). This patch is
wrong.
device_register()
-> device_initialize()
-> kobject_init()
-> kobject_init_internal()
-> kref_init(&kobj->kref); //set
kref->refcount to 1
^^^^^^^^^^^^^^^^^^^^^
Sorry, I missed the initialization of refcount in device_initialize,
but I’m confused about the branch logic for bus_registered. Why isn’t
free(bus) executed when bus_registered == true? My understanding is
that the kobject_cleanup operation triggered when refcount reaches zero
does not clean up the allocated bus. Could you clarify this further?
1020 dev_set_name(&bus->dev, "%04x:%02x", pci_domain_nr(bus),
bus->number);
^^^^^^^^^^^^^^^^^^^^
//device name is allocated, and should be freed when
device_register() is failed.
1021 name = dev_name(&bus->dev);
1022
1023 err = device_register(&bus->dev);
1024 bus_registered = true;
1025 if (err)
1026 goto unregister;
[...]
1117 if (bus_registered)
1118 put_device(&bus->dev);
^^^^^^^^^^^^^^^^
// decrement reference count to zero and
call release_pcibus_dev() to free bus.
// And call kfree_const() to free device
name in kobject_cleanup().
1119 else
1120 kfree(bus);
Commit 804443c1f278 fixes the memory leak of 'name' and consistent with
the annotation of device_degister():
'* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up the
* reference initialized in this function instead.'
Su Hui
