On Mon, 2025-07-21 at 11:47 +0200, Nam Cao wrote:
> rv_reactor has a reference counter to ensure it is not removed while
> monitors are still using it.
>
> However, this is futile, as __exit functions are not expected to fail
> and
> will proceed normally despite rv_unregister_reactor() returning an
> error.
>
> At the moment, reactors do not support being built as modules,
> therefore
> they are never removed and the reference counters are not necessary.
>
> If we support building RV reactors as modules in the future,
> MODULE_SOFTDEP should be used instead of a custom implementation.
>
Mmh, I'm not understanding how, let's assume I create a custom reactor
as a kernel module and I want to use it on existing models (built in or
modules themselves), I'd do.
insmod myreactor
echo myreactor > mymodel/reactors
rmmod myreactor
## I want this one to fail because the reactor is in use
echo nop > mymodel/reactors
rmmod myreactor
# now it can succeed
How is MODULE_SOFTDEP helping in this scenario?
Am I missing something here?
Thanks,
Gabriele
> Remove this reference counter.
>
> Signed-off-by: Nam Cao <nam...@linutronix.de>
> ---
> include/linux/rv.h | 2 --
> kernel/trace/rv/rv.c | 1 -
> kernel/trace/rv/rv.h | 6 ------
> kernel/trace/rv/rv_reactors.c | 33 ++-------------------------------
> 4 files changed, 2 insertions(+), 40 deletions(-)
>
> diff --git a/include/linux/rv.h b/include/linux/rv.h
> index c22c9b8c1567..2f867d6f72ba 100644
> --- a/include/linux/rv.h
> +++ b/include/linux/rv.h
> @@ -91,8 +91,6 @@ struct rv_reactor {
> const char *description;
> __printf(1, 2) void (*react)(const char *msg, ...);
> struct list_head list;
> - /* protected by the monitor interface lock */
> - int counter;
> };
> #endif
>
> diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
> index 6c0be2fdc52d..6c8498743b98 100644
> --- a/kernel/trace/rv/rv.c
> +++ b/kernel/trace/rv/rv.c
> @@ -769,7 +769,6 @@ static const struct file_operations
> monitoring_on_fops = {
>
> static void destroy_monitor_dir(struct rv_monitor *mon)
> {
> - reactor_cleanup_monitor(mon);
> rv_remove(mon->root_d);
> }
>
> diff --git a/kernel/trace/rv/rv.h b/kernel/trace/rv/rv.h
> index 8c38f9dd41bc..1485a70c1bf4 100644
> --- a/kernel/trace/rv/rv.h
> +++ b/kernel/trace/rv/rv.h
> @@ -31,7 +31,6 @@ bool rv_is_nested_monitor(struct rv_monitor *mon);
>
> #ifdef CONFIG_RV_REACTORS
> int reactor_populate_monitor(struct rv_monitor *mon);
> -void reactor_cleanup_monitor(struct rv_monitor *mon);
> int init_rv_reactors(struct dentry *root_dir);
> #else
> static inline int reactor_populate_monitor(struct rv_monitor *mon)
> @@ -39,11 +38,6 @@ static inline int reactor_populate_monitor(struct
> rv_monitor *mon)
> return 0;
> }
>
> -static inline void reactor_cleanup_monitor(struct rv_monitor *mon)
> -{
> - return;
> -}
> -
> static inline int init_rv_reactors(struct dentry *root_dir)
> {
> return 0;
> diff --git a/kernel/trace/rv/rv_reactors.c
> b/kernel/trace/rv/rv_reactors.c
> index 2c7909e6d0e7..a8e849e6cd85 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -172,10 +172,6 @@ static void monitor_swap_reactors_single(struct
> rv_monitor *mon,
> if (monitor_enabled)
> rv_disable_monitor(mon);
>
> - /* swap reactor's usage */
> - mon->reactor->counter--;
> - reactor->counter++;
> -
> mon->reactor = reactor;
> mon->reacting = reacting;
> mon->react = reactor->react;
> @@ -343,23 +339,10 @@ int rv_register_reactor(struct rv_reactor
> *reactor)
> */
> int rv_unregister_reactor(struct rv_reactor *reactor)
> {
> - int ret = 0;
> -
> mutex_lock(&rv_interface_lock);
> -
> - if (!reactor->counter) {
> - list_del(&reactor->list);
> - } else {
> - printk(KERN_WARNING
> - "rv: the rv_reactor %s is in use by %d
> monitor(s)\n",
> - reactor->name, reactor->counter);
> - printk(KERN_WARNING "rv: the rv_reactor %s cannot be
> removed\n",
> - reactor->name);
> - ret = -EBUSY;
> - }
> -
> + list_del(&reactor->list);
> mutex_unlock(&rv_interface_lock);
> - return ret;
> + return 0;
> }
>
> /*
> @@ -456,23 +439,11 @@ int reactor_populate_monitor(struct rv_monitor
> *mon)
> * Configure as the rv_nop reactor.
> */
> mon->reactor = get_reactor_rdef_by_name("nop");
> - mon->reactor->counter++;
> mon->reacting = false;
>
> return 0;
> }
>
> -/**
> - * reactor_cleanup_monitor - cleanup a monitor reference
> - * @mon: the monitor.
> - */
> -void reactor_cleanup_monitor(struct rv_monitor *mon)
> -{
> - lockdep_assert_held(&rv_interface_lock);
> - mon->reactor->counter--;
> - WARN_ON_ONCE(mon->reactor->counter < 0);
> -}
> -
> /*
> * Nop reactor register
> */
