Em Thu, 5 May 2022 23:35:29 +0200 Andi Shyti andi.shyti@linux.intel.com escreveu:

Hi Mauro,

[...]

...

+static int ref_module_dependency(struct module *mod, struct module *this) +{

• int ret;
• if (!this || !this->name)

• return -EINVAL;
  

• if (mod == this)

• return 0;
  

• mutex_lock(&module_mutex);
• ret = ref_module(this, mod);

+#ifdef CONFIG_MODULE_UNLOAD

• if (ret)

• goto ret;
  

• ret = sysfs_create_link(mod->holders_dir,

• 		&this->mkobj.kobj, this->name);
  

+#endif

+ret:

• mutex_unlock(&module_mutex);
• return ret;

+}

/* Clear the unload stuff of the module. */ static void module_unload_free(struct module *mod) { @@ -841,24 +886,16 @@ void __module_get(struct module *module) } EXPORT_SYMBOL(__module_get);

-bool try_module_get(struct module *module) +bool try_module_get_owner(struct module *module, struct module *this) {

• bool ret = true;

• int ret = __try_module_get(module);

• if (module) {

• preempt_disable();
  

• /* Note: here, we can fail to get a reference */
  

• if (likely(module_is_live(module) &&
  

• 	   atomic_inc_not_zero(&module->refcnt) != 0))
  

• 	trace_module_get(module, _RET_IP_);
  

• else
  

• 	ret = false;
  

• if (ret)

• ref_module_dependency(module, this);  
  

do we care about the return value here?

I don't think it should care about the return value, as a failure to create a sysfs node for the holder or to add it to the holders list is not fatal: modules can still continue working without that.

Also, I opted to be conservative here: currently, not creating these doesn't cause try_module_get() to fail. I'm not sure what would be the impact if this starts to fail.

So, right now, I'm opting to just ignore the return value. Perhaps in the future this could a warning (similarly to what sysfs create link does).

Regards, Mauro

Andi

...

• preempt_enable();
  

• } return ret;

} -EXPORT_SYMBOL(try_module_get); +EXPORT_SYMBOL(try_module_get_owner);

void module_put(struct module *module) { -- 2.35.1

Thanks, Mauro