On 2/11/25 22:48, Shyam Saini wrote: > In the unlikely event of the allocation failing, it is better to let > the machine boot with a not fully populated sysfs than to kill it with > this BUG_ON(). All callers are already prepared for > lookup_or_create_module_kobject() returning NULL. > > This is also preparation for calling this function from non __init > code, where using BUG_ON for allocation failure handling is not > acceptable.
I think some error reporting should be cleaned up here. The current situation is that locate_module_kobject() can fail in several cases and all these situations are loudly reported by the function, either by BUG_ON() or pr_crit(). Consistently with that, both its current callers version_sysfs_builtin() and kernel_add_sysfs_param() don't do any reporting if locate_module_kobject() fails; they simply return. The series seems to introduce two somewhat suboptimal cases. With this patch, when either version_sysfs_builtin() or kernel_add_sysfs_param() calls lookup_or_create_module_kobject() and it fails because of a potential kzalloc() error, the problem is silently ignored. Similarly, in the patch #4, when module_add_driver() calls lookup_or_create_module_kobject() and the function fails, the problem may or may not be reported, depending on the error. I'd suggest something as follows: * Drop the pr_crit() reporting in lookup_or_create_module_kobject(). * Have version_sysfs_builtin() and kernel_add_sysfs_param() log an error when lookup_or_create_module_kobject() fails. Using BUG_ON() might be appropriate, as that is already what is used in kernel_add_sysfs_param()? * Update module_add_driver() to propagate any error from lookup_or_create_module_kobject() up the stack. -- Thanks, Petr
