[PATCH v2 0/2] rv: Tidy up with auto-cleanup

[PATCH v2 0/2] rv: Tidy up with auto-cleanup

[Nam Cao]

This series tidies up rv code with __free and lock guard. This is motivated
by a bug:

https://lore.kernel.org/all/20250903065112.1878330-1-zhen.ni@easystack.cn/

which was caused by a missing mutex_unlock. This series makes such bug less
likely in the future, and also reduces some code lines.

Nam Cao (2):
  rv: Convert to use lock guard
  rv: Convert to use __free

 kernel/trace/rv/rv.c          | 102 +++++++++++++---------------------
 kernel/trace/rv/rv.h          |   6 +-
 kernel/trace/rv/rv_reactors.c |  58 +++++++------------
 kernel/trace/trace.c          |   3 +-
 4 files changed, 66 insertions(+), 103 deletions(-)

-- 
2.51.0

[PATCH v2 1/2] rv: Convert to use lock guard

[Nam Cao]

Convert to use lock guard to tidy up the code.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
v2: no change
---
 kernel/trace/rv/rv.c          | 39 ++++++++++++-----------------------
 kernel/trace/rv/rv_reactors.c | 26 +++++++----------------
 2 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
index 43e9ea473cda..b1059a3cf4fa 100644
--- a/kernel/trace/rv/rv.c
+++ b/kernel/trace/rv/rv.c
@@ -375,15 +375,13 @@ static ssize_t monitor_enable_write_data(struct file *filp, const char __user *u
 	if (retval)
 		return retval;
 
-	mutex_lock(&rv_interface_lock);
+	guard(mutex)(&rv_interface_lock);
 
 	if (val)
 		retval = rv_enable_monitor(mon);
 	else
 		retval = rv_disable_monitor(mon);
 
-	mutex_unlock(&rv_interface_lock);
-
 	return retval ? : count;
 }
 
@@ -568,7 +566,7 @@ static void disable_all_monitors(void)
 	struct rv_monitor *mon;
 	int enabled = 0;
 
-	mutex_lock(&rv_interface_lock);
+	guard(mutex)(&rv_interface_lock);
 
 	list_for_each_entry(mon, &rv_monitors_list, list)
 		enabled += __rv_disable_monitor(mon, false);
@@ -581,8 +579,6 @@ static void disable_all_monitors(void)
 		 */
 		tracepoint_synchronize_unregister();
 	}
-
-	mutex_unlock(&rv_interface_lock);
 }
 
 static int enabled_monitors_open(struct inode *inode, struct file *file)
@@ -623,7 +619,7 @@ static ssize_t enabled_monitors_write(struct file *filp, const char __user *user
 	if (!len)
 		return count;
 
-	mutex_lock(&rv_interface_lock);
+	guard(mutex)(&rv_interface_lock);
 
 	retval = -EINVAL;
 
@@ -644,13 +640,11 @@ static ssize_t enabled_monitors_write(struct file *filp, const char __user *user
 		else
 			retval = rv_disable_monitor(mon);
 
-		if (!retval)
-			retval = count;
-
-		break;
+		if (retval)
+			return retval;
+		return count;
 	}
 
-	mutex_unlock(&rv_interface_lock);
 	return retval;
 }
 
@@ -737,7 +731,7 @@ static ssize_t monitoring_on_write_data(struct file *filp, const char __user *us
 	if (retval)
 		return retval;
 
-	mutex_lock(&rv_interface_lock);
+	guard(mutex)(&rv_interface_lock);
 
 	if (val)
 		turn_monitoring_on_with_reset();
@@ -750,8 +744,6 @@ static ssize_t monitoring_on_write_data(struct file *filp, const char __user *us
 	 */
 	tracepoint_synchronize_unregister();
 
-	mutex_unlock(&rv_interface_lock);
-
 	return count;
 }
 
@@ -784,28 +776,26 @@ int rv_register_monitor(struct rv_monitor *monitor, struct rv_monitor *parent)
 		return -EINVAL;
 	}
 
-	mutex_lock(&rv_interface_lock);
+	guard(mutex)(&rv_interface_lock);
 
 	list_for_each_entry(r, &rv_monitors_list, list) {
 		if (strcmp(monitor->name, r->name) == 0) {
 			pr_info("Monitor %s is already registered\n", monitor->name);
-			retval = -EEXIST;
-			goto out_unlock;
+			return -EEXIST;
 		}
 	}
 
 	if (parent && rv_is_nested_monitor(parent)) {
 		pr_info("Parent monitor %s is already nested, cannot nest further\n",
 			parent->name);
-		retval = -EINVAL;
-		goto out_unlock;
+		return -EINVAL;
 	}
 
 	monitor->parent = parent;
 
 	retval = create_monitor_dir(monitor, parent);
 	if (retval)
-		goto out_unlock;
+		return retval;
 
 	/* keep children close to the parent for easier visualisation */
 	if (parent)
@@ -813,9 +803,7 @@ int rv_register_monitor(struct rv_monitor *monitor, struct rv_monitor *parent)
 	else
 		list_add_tail(&monitor->list, &rv_monitors_list);
 
-out_unlock:
-	mutex_unlock(&rv_interface_lock);
-	return retval;
+	return 0;
 }
 
 /**
@@ -826,13 +814,12 @@ int rv_register_monitor(struct rv_monitor *monitor, struct rv_monitor *parent)
  */
 int rv_unregister_monitor(struct rv_monitor *monitor)
 {
-	mutex_lock(&rv_interface_lock);
+	guard(mutex)(&rv_interface_lock);
 
 	rv_disable_monitor(monitor);
 	list_del(&monitor->list);
 	destroy_monitor_dir(monitor);
 
-	mutex_unlock(&rv_interface_lock);
 	return 0;
 }
 
diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index d32859fec238..37d1d37a27a3 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -232,9 +232,7 @@ monitor_reactors_write(struct file *file, const char __user *user_buf,
 	seq_f = file->private_data;
 	mon = seq_f->private;
 
-	mutex_lock(&rv_interface_lock);
-
-	retval = -EINVAL;
+	guard(mutex)(&rv_interface_lock);
 
 	list_for_each_entry(reactor, &rv_reactors_list, list) {
 		if (strcmp(ptr, reactor->name) != 0)
@@ -242,13 +240,10 @@ monitor_reactors_write(struct file *file, const char __user *user_buf,
 
 		monitor_swap_reactors(mon, reactor);
 
-		retval = count;
-		break;
+		return count;
 	}
 
-	mutex_unlock(&rv_interface_lock);
-
-	return retval;
+	return -EINVAL;
 }
 
 /*
@@ -309,18 +304,14 @@ static int __rv_register_reactor(struct rv_reactor *reactor)
  */
 int rv_register_reactor(struct rv_reactor *reactor)
 {
-	int retval = 0;
-
 	if (strlen(reactor->name) >= MAX_RV_REACTOR_NAME_SIZE) {
 		pr_info("Reactor %s has a name longer than %d\n",
 			reactor->name, MAX_RV_MONITOR_NAME_SIZE);
 		return -EINVAL;
 	}
 
-	mutex_lock(&rv_interface_lock);
-	retval = __rv_register_reactor(reactor);
-	mutex_unlock(&rv_interface_lock);
-	return retval;
+	guard(mutex)(&rv_interface_lock);
+	return __rv_register_reactor(reactor);
 }
 
 /**
@@ -331,9 +322,8 @@ int rv_register_reactor(struct rv_reactor *reactor)
  */
 int rv_unregister_reactor(struct rv_reactor *reactor)
 {
-	mutex_lock(&rv_interface_lock);
+	guard(mutex)(&rv_interface_lock);
 	list_del(&reactor->list);
-	mutex_unlock(&rv_interface_lock);
 	return 0;
 }
 
@@ -389,7 +379,7 @@ static ssize_t reacting_on_write_data(struct file *filp, const char __user *user
 	if (retval)
 		return retval;
 
-	mutex_lock(&rv_interface_lock);
+	guard(mutex)(&rv_interface_lock);
 
 	if (val)
 		turn_reacting_on();
@@ -402,8 +392,6 @@ static ssize_t reacting_on_write_data(struct file *filp, const char __user *user
 	 */
 	tracepoint_synchronize_unregister();
 
-	mutex_unlock(&rv_interface_lock);
-
 	return count;
 }
 
-- 
2.51.0

[PATCH v2 2/2] rv: Convert to use __free

[Nam Cao]

Convert to use __free to tidy up the code.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
v2: address Gabriele's comments:
  - simply with no_free_ptr()
  - add back a removed printk()
  - add a NULL check to DEFINE_FREE(rv_remove, ...)
  - add a missing __free()
---
 kernel/trace/rv/rv.c          | 63 +++++++++++++++--------------------
 kernel/trace/rv/rv.h          |  6 ++--
 kernel/trace/rv/rv_reactors.c | 32 ++++++++----------
 kernel/trace/trace.c          |  3 +-
 4 files changed, 46 insertions(+), 58 deletions(-)

diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
index b1059a3cf4fa..ee4e68102f17 100644
--- a/kernel/trace/rv/rv.c
+++ b/kernel/trace/rv/rv.c
@@ -420,35 +420,27 @@ static const struct file_operations interface_desc_fops = {
 static int create_monitor_dir(struct rv_monitor *mon, struct rv_monitor *parent)
 {
 	struct dentry *root = parent ? parent->root_d : get_monitors_root();
-	const char *name = mon->name;
+	struct dentry *dir __free(rv_remove) = rv_create_dir(mon->name, root);
 	struct dentry *tmp;
 	int retval;
 
-	mon->root_d = rv_create_dir(name, root);
-	if (!mon->root_d)
+	if (!dir)
 		return -ENOMEM;
 
-	tmp = rv_create_file("enable", RV_MODE_WRITE, mon->root_d, mon, &interface_enable_fops);
-	if (!tmp) {
-		retval = -ENOMEM;
-		goto out_remove_root;
-	}
+	tmp = rv_create_file("enable", RV_MODE_WRITE, dir, mon, &interface_enable_fops);
+	if (!tmp)
+		return -ENOMEM;
 
-	tmp = rv_create_file("desc", RV_MODE_READ, mon->root_d, mon, &interface_desc_fops);
-	if (!tmp) {
-		retval = -ENOMEM;
-		goto out_remove_root;
-	}
+	tmp = rv_create_file("desc", RV_MODE_READ, dir, mon, &interface_desc_fops);
+	if (!tmp)
+		return -ENOMEM;
 
-	retval = reactor_populate_monitor(mon);
+	retval = reactor_populate_monitor(mon, dir);
 	if (retval)
-		goto out_remove_root;
+		return retval;
 
+	mon->root_d = no_free_ptr(dir);
 	return 0;
-
-out_remove_root:
-	rv_remove(mon->root_d);
-	return retval;
 }
 
 /*
@@ -827,39 +819,36 @@ int __init rv_init_interface(void)
 {
 	struct dentry *tmp;
 	int retval;
+	struct dentry *root_dir __free(rv_remove) = rv_create_dir("rv", NULL);
 
-	rv_root.root_dir = rv_create_dir("rv", NULL);
-	if (!rv_root.root_dir)
-		goto out_err;
+	if (!root_dir)
+		return 1;
 
-	rv_root.monitors_dir = rv_create_dir("monitors", rv_root.root_dir);
+	rv_root.monitors_dir = rv_create_dir("monitors", root_dir);
 	if (!rv_root.monitors_dir)
-		goto out_err;
+		return 1;
 
-	tmp = rv_create_file("available_monitors", RV_MODE_READ, rv_root.root_dir, NULL,
+	tmp = rv_create_file("available_monitors", RV_MODE_READ, root_dir, NULL,
 			     &available_monitors_ops);
 	if (!tmp)
-		goto out_err;
+		return 1;
 
-	tmp = rv_create_file("enabled_monitors", RV_MODE_WRITE, rv_root.root_dir, NULL,
+	tmp = rv_create_file("enabled_monitors", RV_MODE_WRITE, root_dir, NULL,
 			     &enabled_monitors_ops);
 	if (!tmp)
-		goto out_err;
+		return 1;
 
-	tmp = rv_create_file("monitoring_on", RV_MODE_WRITE, rv_root.root_dir, NULL,
+	tmp = rv_create_file("monitoring_on", RV_MODE_WRITE, root_dir, NULL,
 			     &monitoring_on_fops);
 	if (!tmp)
-		goto out_err;
-	retval = init_rv_reactors(rv_root.root_dir);
+		return 1;
+	retval = init_rv_reactors(root_dir);
 	if (retval)
-		goto out_err;
+		return 1;
 
 	turn_monitoring_on();
 
-	return 0;
+	rv_root.root_dir = no_free_ptr(root_dir);
 
-out_err:
-	rv_remove(rv_root.root_dir);
-	printk(KERN_ERR "RV: Error while creating the RV interface\n");
-	return 1;
+	return 0;
 }
diff --git a/kernel/trace/rv/rv.h b/kernel/trace/rv/rv.h
index 1485a70c1bf4..2c0f51ff9d5c 100644
--- a/kernel/trace/rv/rv.h
+++ b/kernel/trace/rv/rv.h
@@ -17,6 +17,8 @@ struct rv_interface {
 #define rv_create_file			tracefs_create_file
 #define rv_remove			tracefs_remove
 
+DEFINE_FREE(rv_remove, struct dentry *, if (_T) rv_remove(_T));
+
 #define MAX_RV_MONITOR_NAME_SIZE	32
 #define MAX_RV_REACTOR_NAME_SIZE	32
 
@@ -30,10 +32,10 @@ bool rv_is_container_monitor(struct rv_monitor *mon);
 bool rv_is_nested_monitor(struct rv_monitor *mon);
 
 #ifdef CONFIG_RV_REACTORS
-int reactor_populate_monitor(struct rv_monitor *mon);
+int reactor_populate_monitor(struct rv_monitor *mon, struct dentry *root);
 int init_rv_reactors(struct dentry *root_dir);
 #else
-static inline int reactor_populate_monitor(struct rv_monitor *mon)
+static inline int reactor_populate_monitor(struct rv_monitor *mon, struct dentry *root)
 {
 	return 0;
 }
diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index 37d1d37a27a3..4a98967d677b 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -404,14 +404,15 @@ static const struct file_operations reacting_on_fops = {
 /**
  * reactor_populate_monitor - creates per monitor reactors file
  * @mon:	The monitor.
+ * @root:	The directory of the monitor.
  *
  * Returns 0 if successful, error otherwise.
  */
-int reactor_populate_monitor(struct rv_monitor *mon)
+int reactor_populate_monitor(struct rv_monitor *mon, struct dentry *root)
 {
 	struct dentry *tmp;
 
-	tmp = rv_create_file("reactors", RV_MODE_WRITE, mon->root_d, mon, &monitor_reactors_ops);
+	tmp = rv_create_file("reactors", RV_MODE_WRITE, root, mon, &monitor_reactors_ops);
 	if (!tmp)
 		return -ENOMEM;
 
@@ -438,30 +439,25 @@ static struct rv_reactor rv_nop = {
 
 int init_rv_reactors(struct dentry *root_dir)
 {
-	struct dentry *available, *reacting;
 	int retval;
 
-	available = rv_create_file("available_reactors", RV_MODE_READ, root_dir, NULL,
-				   &available_reactors_ops);
-	if (!available)
-		goto out_err;
+	struct dentry *available __free(rv_remove) =
+		rv_create_file("available_reactors", RV_MODE_READ, root_dir,
+				NULL, &available_reactors_ops);
 
-	reacting = rv_create_file("reacting_on", RV_MODE_WRITE, root_dir, NULL, &reacting_on_fops);
-	if (!reacting)
-		goto rm_available;
+	struct dentry *reacting __free(rv_remove) =
+		rv_create_file("reacting_on", RV_MODE_WRITE, root_dir, NULL, &reacting_on_fops);
+
+	if (!reacting || !available)
+		return -ENOMEM;
 
 	retval = __rv_register_reactor(&rv_nop);
 	if (retval)
-		goto rm_reacting;
+		return retval;
 
 	turn_reacting_on();
 
+	retain_and_null_ptr(available);
+	retain_and_null_ptr(reacting);
 	return 0;
-
-rm_reacting:
-	rv_remove(reacting);
-rm_available:
-	rv_remove(available);
-out_err:
-	return -ENOMEM;
 }
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d1e527cf2aae..cd209598d670 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10640,7 +10640,8 @@ static __init int tracer_init_tracefs(void)
 		tracer_init_tracefs_work_func(NULL);
 	}
 
-	rv_init_interface();
+	if (rv_init_interface())
+		pr_err("RV: Error while creating the RV interface\n");
 
 	return 0;
 }
-- 
2.51.0

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Steven Rostedt]

On Mon, 17 Nov 2025 09:06:02 +0000
Nam Cao <namcao@linutronix.de> wrote:

> @@ -644,13 +640,11 @@ static ssize_t enabled_monitors_write(struct file *filp, const char __user *user
>  		else
>  			retval = rv_disable_monitor(mon);
>  
> -		if (!retval)
> -			retval = count;
> -
> -		break;
> +		if (retval)
> +			return retval;
> +		return count;

No biggy, but I wonder if this would look better as:

		return retval ? : count;

-- Steve

>  	}
>  
> -	mutex_unlock(&rv_interface_lock);
>  	return retval;
>  }
>  

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Gabriele Monaco]

On Tue, 2025-11-25 at 14:57 -0500, Steven Rostedt wrote:
> On Mon, 17 Nov 2025 09:06:02 +0000
> Nam Cao <namcao@linutronix.de> wrote:
> 
> > @@ -644,13 +640,11 @@ static ssize_t enabled_monitors_write(struct file
> > *filp, const char __user *user
> >  		else
> >  			retval = rv_disable_monitor(mon);
> >  
> > -		if (!retval)
> > -			retval = count;
> > -
> > -		break;
> > +		if (retval)
> > +			return retval;
> > +		return count;
> 
> No biggy, but I wonder if this would look better as:
> 
> 		return retval ? : count;
> 

Tried both patches and they look fine to me.

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>

Nam, feel free to send an updated version if you want to apply Steve's
suggestion or keep the patch like this.

Steve, this is the only remaining change before the merge window, unless you
prefer to keep it for the next round, I'm going to send a small pull requests
with those two patches alone.

Thanks,
Gabriele

> -- Steve
> 
> >  	}
> >  
> > -	mutex_unlock(&rv_interface_lock);
> >  	return retval;
> >  }
> >  

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Nam Cao]

On Tue, 2025-11-25 at 14:57 -0500, Steven Rostedt wrote:
> On Mon, 17 Nov 2025 09:06:02 +0000
> Nam Cao <namcao@linutronix.de> wrote:
> 
> > @@ -644,13 +640,11 @@ static ssize_t enabled_monitors_write(struct file
> > *filp, const char __user *user
> >  		else
> >  			retval = rv_disable_monitor(mon);
> >  
> > -		if (!retval)
> > -			retval = count;
> > -
> > -		break;
> > +		if (retval)
> > +			return retval;
> > +		return count;
> 
> No biggy, but I wonder if this would look better as:
> 
> 		return retval ? : count;

Unless you really prefer it this way, I would rather not. The first time
I saw this syntax, it confused the hell out of me. Took me some time
scratching my head until I figured out that it is a GNU extension.

I prefer to stay with the C standard unless there is major benefit not
to.

Nam

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Gabriele Monaco]

On Wed, 2025-11-26 at 09:36 +0100, Nam Cao wrote:
> On Tue, 2025-11-25 at 14:57 -0500, Steven Rostedt wrote:
> > On Mon, 17 Nov 2025 09:06:02 +0000
> > Nam Cao <namcao@linutronix.de> wrote:
> > 
> > > @@ -644,13 +640,11 @@ static ssize_t enabled_monitors_write(struct file
> > > *filp, const char __user *user
> > >  		else
> > >  			retval = rv_disable_monitor(mon);
> > >  
> > > -		if (!retval)
> > > -			retval = count;
> > > -
> > > -		break;
> > > +		if (retval)
> > > +			return retval;
> > > +		return count;
> > 
> > No biggy, but I wonder if this would look better as:
> > 
> > 		return retval ? : count;
> 
> Unless you really prefer it this way, I would rather not. The first time
> I saw this syntax, it confused the hell out of me. Took me some time
> scratching my head until I figured out that it is a GNU extension.
> 
> I prefer to stay with the C standard unless there is major benefit not
> to.

To be fair, I find it a bit obscure as well, although it's frequently used
within the kernel.

Let's not change it then.

Thanks,
Gabriele

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Steven Rostedt]

On Wed, 26 Nov 2025 08:27:42 +0100
Gabriele Monaco <gmonaco@redhat.com> wrote:

> Steve, this is the only remaining change before the merge window, unless you
> prefer to keep it for the next round, I'm going to send a small pull requests
> with those two patches alone.

Feel free to send the small pull request.

Note, it's a US holiday tomorrow and Friday, where I will probably not be
working. Everything will need to wait until Monday, which means the merge
window will likely be open.

-- Steve

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Gabriele Monaco]

On Wed, 2025-11-26 at 09:51 -0500, Steven Rostedt wrote:
> On Wed, 26 Nov 2025 08:27:42 +0100
> Gabriele Monaco <gmonaco@redhat.com> wrote:
> 
> > Steve, this is the only remaining change before the merge window, unless you
> > prefer to keep it for the next round, I'm going to send a small pull
> > requests with those two patches alone.
> 
> Feel free to send the small pull request.
> 
> Note, it's a US holiday tomorrow and Friday, where I will probably not be
> working. Everything will need to wait until Monday, which means the merge
> window will likely be open.

I can send it quickly since you already had a look at those two patches.
It isn't a big problem you won't have time, though. It can stay for the next
round.

Thanks,
Gabriele

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Steven Rostedt]

On Wed, 26 Nov 2025 10:33:51 +0100
Gabriele Monaco <gmonaco@redhat.com> wrote:

> > > No biggy, but I wonder if this would look better as:
> > > 
> > > 		return retval ? : count;  
> > 
> > Unless you really prefer it this way, I would rather not. The first time
> > I saw this syntax, it confused the hell out of me. Took me some time
> > scratching my head until I figured out that it is a GNU extension.
> > 
> > I prefer to stay with the C standard unless there is major benefit not
> > to.  
> 
> To be fair, I find it a bit obscure as well, although it's frequently used
> within the kernel.
> 
> Let's not change it then.

As I said, "No biggy". I personally like that notation better. But that's
my own personal choice. I won't push that on other ;-)

-- Steve

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Steven Rostedt]

On Wed, 26 Nov 2025 16:22:17 +0100
Gabriele Monaco <gmonaco@redhat.com> wrote:

> I can send it quickly since you already had a look at those two patches.
> It isn't a big problem you won't have time, though. It can stay for the next
> round.

Did you send them?

-- Steve

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Gabriele Monaco]

On Mon, 2025-12-01 at 10:24 -0500, Steven Rostedt wrote:
> On Wed, 26 Nov 2025 16:22:17 +0100
> Gabriele Monaco <gmonaco@redhat.com> wrote:
> 
> > I can send it quickly since you already had a look at those two patches.
> > It isn't a big problem you won't have time, though. It can stay for the next
> > round.
> 
> Did you send them?

I'm stupid, I sent it [1] but the script somehow didn't add you in the To: field
and I didn't notice..

I'd say since the merge window is open, this change can wait.

Sorry for that.

Gabriele

[1] - https://lore.kernel.org/lkml/20251126152253.464350-1-gmonaco@redhat.com

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Nam Cao]

Steven Rostedt <rostedt@goodmis.org> writes:
> On Wed, 26 Nov 2025 16:22:17 +0100
> Gabriele Monaco <gmonaco@redhat.com> wrote:
>
>> I can send it quickly since you already had a look at those two patches.
>> It isn't a big problem you won't have time, though. It can stay for the next
>> round.
>
> Did you send them?

Looks like Gabriele forgot to include you as a recipient:
https://lore.kernel.org/lkml/20251126152253.464350-1-gmonaco@redhat.com/

Nam

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Steven Rostedt]

On Mon, 01 Dec 2025 16:28:21 +0100
Gabriele Monaco <gmonaco@redhat.com> wrote:

> I'm stupid, I sent it [1] but the script somehow didn't add you in the To: field
> and I didn't notice..
> 
> I'd say since the merge window is open, this change can wait.
> 
> Sorry for that.
> 
> Gabriele
> 
> [1] - https://lore.kernel.org/lkml/20251126152253.464350-1-gmonaco@redhat.com

I can still pull it and run it through my tests tonight and push it later this week.

It's just clean up code. Linus doesn't get too upset if simple cleanup code
gets added just before the merge window. It's new features that he doesn't
like added late in the game.

-- Steve

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Steven Rostedt]

On Mon, 1 Dec 2025 20:38:16 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 01 Dec 2025 16:28:21 +0100
> Gabriele Monaco <gmonaco@redhat.com> wrote:
> 
> > I'm stupid, I sent it [1] but the script somehow didn't add you in the To: field
> > and I didn't notice..
> > 
> > I'd say since the merge window is open, this change can wait.
> > 
> > Sorry for that.
> > 
> > Gabriele
> > 
> > [1] - https://lore.kernel.org/lkml/20251126152253.464350-1-gmonaco@redhat.com  
> 
> I can still pull it and run it through my tests tonight and push it later this week.
> 
> It's just clean up code. Linus doesn't get too upset if simple cleanup code
> gets added just before the merge window. It's new features that he doesn't
> like added late in the game.

After pulling it, I take it back ;-)

It's best to always use your latest branch that you did your last pull on
during a release cycle.

I see you based these changes on v6.18-rc7, but your previous pull was
based on v6.18-rc5. If I were to pull this in, it gets a bit spaghetti like
in the merges.

  v6.18-rc5 -> pull1
  merge <- v6.18-rc7 <- pull2

Where now we have changes between rc5 and rc7.  Was there a reason you
based on top of rc7 and not use your last pull?

It's fine sending urgent patches this way, as these tags are in Linus's
tree and it's just new changes being added. But for a subsytem tree, it's
best not to pull in Linus's tree unless there's a good reason to do that.

What I can do is simply pull the patches on top of your last patch
directly, and keep the history clean for my pull request to Linus.

-- Steve

Re: [PATCH v2 1/2] rv: Convert to use lock guard

[Gabriele Monaco]

> On Mon, 1 Dec 2025 20:38:16 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> It's best to always use your latest branch that you did your last pull on
> during a release cycle.
>
> I see you based these changes on v6.18-rc7, but your previous pull was
> based on v6.18-rc5. If I were to pull this in, it gets a bit spaghetti like
> in the merges.
>
>   v6.18-rc5 -> pull1
>   merge <- v6.18-rc7 <- pull2
>
> Where now we have changes between rc5 and rc7.  Was there a reason you
> based on top of rc7 and not use your last pull?
>
> It's fine sending urgent patches this way, as these tags are in Linus's
> tree and it's just new changes being added. But for a subsytem tree, it's
> best not to pull in Linus's tree unless there's a good reason to do that.

Yeah got it, there was no valid reason for that rebase, I'm going to keep it
stable next time or write it explicitly otherwise.

> What I can do is simply pull the patches on top of your last patch
> directly, and keep the history clean for my pull request to Linus.

I'm pushed the tag rebased on -rc5 (actually on the last PR) just in case it
makes your life easier:


The following changes since commit 69d8895cb9a9f6450374577af8584c2e21cb5a9f:

  rv: Add explicit lockdep context for reactors (2025-11-11 13:18:56 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gmonaco/linux.git rv-6.19-next-2-rc5

for you to fetch changes up to b30f635bb6492f02f2f704b46d898679371015cb:

  rv: Convert to use __free (2025-12-02 07:28:32 +0100)


Thanks,
Gabriele