From: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
To: linux-modules@vger.kernel.org
Cc: Daniel Gomez <da.gomez@samsung.com>,
Luis Chamberlain <mcgrof@kernel.org>,
Petr Pavlu <petr.pavlu@suse.com>,
Sami Tolvanen <samitolvanen@google.com>
Subject: [PATCH] module: Use rcuref_t for module::refcnt.
Date: Sun, 9 Mar 2025 13:19:14 +0100 [thread overview]
Message-ID: <20250309121914.pN6eBMvI@breakpoint.cc> (raw)
By using rcuref_t the atomic_inc_not_zero() and atomic_dec_if_positive()
can disappear. By design rcuref_t does not allow decrementing past the
"0" reference and increment once it has been released. It will spill
warnings if there are more puts than initial gets.
This also makes the put/ get attempt in try_release_module_ref() a
little simpler. Should the get in try_release_module_ref() fail then
there should be another warning originating from module_put() which is
the only race I can imagine.
Use rcuref_t for module::refcnt.
Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
include/linux/module.h | 2 +-
include/trace/events/module.h | 2 +-
kernel/module/main.c | 51 ++++++++++++++++++-----------------
3 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index d9a5183a9fe71..b473a329f1927 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -581,7 +581,7 @@ struct module {
/* Destruction function. */
void (*exit)(void);
- atomic_t refcnt;
+ rcuref_t refcnt;
#endif
#ifdef CONFIG_CONSTRUCTORS
diff --git a/include/trace/events/module.h b/include/trace/events/module.h
index e5a006be9dc66..cdb88bb4bc24a 100644
--- a/include/trace/events/module.h
+++ b/include/trace/events/module.h
@@ -81,7 +81,7 @@ DECLARE_EVENT_CLASS(module_refcnt,
TP_fast_assign(
__entry->ip = ip;
- __entry->refcnt = atomic_read(&mod->refcnt);
+ __entry->refcnt = rcuref_read(&mod->refcnt);
__assign_str(name);
),
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 3dbe230984336..15b26e1431f59 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -570,17 +570,15 @@ EXPORT_TRACEPOINT_SYMBOL(module_get);
/* Init the unload section of the module. */
static int module_unload_init(struct module *mod)
{
- /*
- * Initialize reference counter to MODULE_REF_BASE.
- * refcnt == 0 means module is going.
- */
- atomic_set(&mod->refcnt, MODULE_REF_BASE);
-
INIT_LIST_HEAD(&mod->source_list);
INIT_LIST_HEAD(&mod->target_list);
- /* Hold reference count during initialization. */
- atomic_inc(&mod->refcnt);
+ /*
+ * Initialize reference counter to MODULE_REF_BASE.
+ * refcnt == 0 means module is going.
+ * Hold reference count during initialization.
+ */
+ rcuref_init(&mod->refcnt, MODULE_REF_BASE + 1);
return 0;
}
@@ -674,25 +672,33 @@ static inline int try_force_unload(unsigned int flags)
}
#endif /* CONFIG_MODULE_FORCE_UNLOAD */
-/* Try to release refcount of module, 0 means success. */
-static int try_release_module_ref(struct module *mod)
+/* Try to release the initial reference of module, true means success. */
+static bool try_release_module_ref(struct module *mod)
{
- int ret;
+ bool ret;
/* Try to decrement refcnt which we set at loading */
- ret = atomic_sub_return(MODULE_REF_BASE, &mod->refcnt);
- BUG_ON(ret < 0);
+ ret = rcuref_put(&mod->refcnt);
if (ret)
- /* Someone can put this right now, recover with checking */
- ret = atomic_add_unless(&mod->refcnt, MODULE_REF_BASE, 0);
+ /* Last reference put, module can go */
+ return true;
- return ret;
+ ret = rcuref_get(&mod->refcnt);
+ if (!ret)
+ /*
+ * Last put failed but can't acquire a reference. This means
+ * the previous owner has put the reference.
+ */
+ return true;
+
+ /* There is still a reference on the module */
+ return false;
}
static int try_stop_module(struct module *mod, int flags, int *forced)
{
/* If it's not unused, quit unless we're forcing. */
- if (try_release_module_ref(mod) != 0) {
+ if (try_release_module_ref(mod) != true) {
*forced = try_force_unload(flags);
if (!(*forced))
return -EWOULDBLOCK;
@@ -714,7 +720,7 @@ static int try_stop_module(struct module *mod, int flags, int *forced)
*/
int module_refcount(struct module *mod)
{
- return atomic_read(&mod->refcnt) - MODULE_REF_BASE;
+ return rcuref_read(&mod->refcnt) - MODULE_REF_BASE;
}
EXPORT_SYMBOL(module_refcount);
@@ -844,7 +850,7 @@ static const struct module_attribute modinfo_refcnt =
void __module_get(struct module *module)
{
if (module) {
- atomic_inc(&module->refcnt);
+ WARN_ON(!rcuref_get(&module->refcnt));
trace_module_get(module, _RET_IP_);
}
}
@@ -857,7 +863,7 @@ bool try_module_get(struct module *module)
if (module) {
/* Note: here, we can fail to get a reference */
if (likely(module_is_live(module) &&
- atomic_inc_not_zero(&module->refcnt) != 0))
+ rcuref_get(&module->refcnt)))
trace_module_get(module, _RET_IP_);
else
ret = false;
@@ -868,11 +874,8 @@ EXPORT_SYMBOL(try_module_get);
void module_put(struct module *module)
{
- int ret;
-
if (module) {
- ret = atomic_dec_if_positive(&module->refcnt);
- WARN_ON(ret < 0); /* Failed to put refcount */
+ WARN_ON(rcuref_put(&module->refcnt));
trace_module_put(module, _RET_IP_);
}
}
--
2.47.2
next reply other threads:[~2025-03-09 12:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-09 12:19 Sebastian Andrzej Siewior [this message]
2025-03-10 14:28 ` [PATCH] module: Use rcuref_t for module::refcnt Petr Pavlu
2025-03-10 21:24 ` Sebastian Andrzej Siewior
2025-03-17 16:33 ` Petr Pavlu
2025-05-12 21:22 ` Sebastian Andrzej Siewior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250309121914.pN6eBMvI@breakpoint.cc \
--to=sebastian@breakpoint.cc \
--cc=da.gomez@samsung.com \
--cc=linux-modules@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=petr.pavlu@suse.com \
--cc=samitolvanen@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).