RCU_BOOST not working for me

RCU_BOOST not working for me

[Joel Fernandes]

Hi,
Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
test it a bit and I don't see the boost happening (thanks to Daniel for idea
of writing a module). Haven't debugged it more yet. Will look more tomorrow.
But below is the kernel module code and it prints a FAIL message to kernel
logs in a few seconds.

I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
is set to 500.

Thoughts?

---8<-----------------------

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c1860d35dc7e..ba34957dff26 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -2,7 +2,7 @@
 #
 # Makefile for misc devices that really don't fit anywhere else.
 #
-
+obj-m += ptest.o
 obj-$(CONFIG_IBM_ASM)		+= ibmasm/
 obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
 obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
new file mode 100644
index 000000000000..76cc9524ccac
--- /dev/null
+++ b/drivers/misc/ptest.c
@@ -0,0 +1,112 @@
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/vmalloc.h>
+#include <linux/kthread.h>
+#include <linux/delay.h>
+
+#define RCU_READER_DELAY 100 //ms
+#define RCU_BLOCKER_DELAY 600 //ms
+
+MODULE_LICENSE("GPL");
+
+struct sched_param {
+	int sched_priority;
+};
+
+int stop_test = 0;
+int test_pass = 1;
+int reader_exit = 0;
+s64 delta_fail;
+
+#define ns_to_ms(delta) (delta / 1000000ULL)
+
+static int rcu_reader(void *a)
+{
+	ktime_t start, end, reader_begin;
+	s64 delta;
+
+	reader_begin = ktime_get();
+
+	while (!kthread_should_stop() && !stop_test) {
+		start = ktime_get();
+		rcu_read_lock();
+		trace_printk("rcu_reader entering RSCS\n");
+		mdelay(RCU_READER_DELAY);
+		trace_printk("rcu_reader exiting RSCS\n");
+		rcu_read_lock();
+		end = ktime_get();
+		delta = ktime_to_ns(ktime_sub(end, start));
+
+		if (delta < 0 || (ns_to_ms(delta) > (2 * RCU_READER_DELAY))) {
+			delta_fail = delta;
+			test_pass = 0;
+			break;
+		}
+
+		// Don't let the rcu_reader() run more than 3s inorder to
+		// not starve the blocker incase reader prio > blocker prio.
+		delta = ktime_to_ns(ktime_sub(end, reader_begin));
+		if (ns_to_ms(delta) > 3000)
+			break;
+	}
+
+	stop_test = 1;
+	reader_exit = 1;
+	pr_err("Exiting reader\n");
+	return 0;
+}
+
+static int rcu_blocker(void *a)
+{
+	int loops = 5;
+
+	while (!kthread_should_stop() && loops-- && !stop_test) {
+		trace_printk("rcu_blocker entering\n");
+		mdelay(RCU_BLOCKER_DELAY);
+		trace_printk("rcu_blocker exiting\n");
+	}
+
+	pr_err("Exiting blocker\n");
+	stop_test = 1;
+
+	// Wait for reader to finish
+	while (!reader_exit)
+		schedule_timeout_uninterruptible(1);
+
+	if (test_pass)
+		pr_err("TEST PASSED\n");
+	else
+		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
+
+	return 0;
+}
+
+static int __init ptest_init(void){
+	struct sched_param params;
+	struct task_struct *reader, *blocker;
+
+	reader = kthread_create(rcu_reader, NULL, "reader");
+	params.sched_priority = 50;
+	sched_setscheduler(reader, SCHED_FIFO, &params);
+	kthread_bind(reader, smp_processor_id());
+
+	blocker = kthread_create(rcu_blocker, NULL, "blocker");
+	params.sched_priority = 60;
+	sched_setscheduler(blocker, SCHED_FIFO, &params);
+	kthread_bind(blocker, smp_processor_id());
+
+	wake_up_process(reader);
+
+	// Let reader run a little
+	mdelay(50);
+
+	wake_up_process(blocker);
+	return 0;
+}
+
+static void __exit ptest_exit(void){
+}
+
+module_init(ptest_init);
+module_exit(ptest_exit);
-- 
2.25.0.341.g760bfbb309-goog

Re: RCU_BOOST not working for me

[Joel Fernandes]

On Fri, Jan 17, 2020 at 04:58:14PM -0500, Joel Fernandes wrote:
> Hi,
> Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
> test it a bit and I don't see the boost happening (thanks to Daniel for idea
> of writing a module). Haven't debugged it more yet. Will look more tomorrow.
> But below is the kernel module code and it prints a FAIL message to kernel
> logs in a few seconds.
> 
> I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
> is set to 500.
> 
> Thoughts?

So this could be because I did not start a grace period which is quite silly.
I am sorry about that. I will add another thread to start grace periods as
well and let you know if I still see a problem.

thanks,

 - Joel


> 
> ---8<-----------------------
> 
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index c1860d35dc7e..ba34957dff26 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -2,7 +2,7 @@
>  #
>  # Makefile for misc devices that really don't fit anywhere else.
>  #
> -
> +obj-m += ptest.o
>  obj-$(CONFIG_IBM_ASM)		+= ibmasm/
>  obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
>  obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
> diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
> new file mode 100644
> index 000000000000..76cc9524ccac
> --- /dev/null
> +++ b/drivers/misc/ptest.c
> @@ -0,0 +1,112 @@
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/vmalloc.h>
> +#include <linux/kthread.h>
> +#include <linux/delay.h>
> +
> +#define RCU_READER_DELAY 100 //ms
> +#define RCU_BLOCKER_DELAY 600 //ms
> +
> +MODULE_LICENSE("GPL");
> +
> +struct sched_param {
> +	int sched_priority;
> +};
> +
> +int stop_test = 0;
> +int test_pass = 1;
> +int reader_exit = 0;
> +s64 delta_fail;
> +
> +#define ns_to_ms(delta) (delta / 1000000ULL)
> +
> +static int rcu_reader(void *a)
> +{
> +	ktime_t start, end, reader_begin;
> +	s64 delta;
> +
> +	reader_begin = ktime_get();
> +
> +	while (!kthread_should_stop() && !stop_test) {
> +		start = ktime_get();
> +		rcu_read_lock();
> +		trace_printk("rcu_reader entering RSCS\n");
> +		mdelay(RCU_READER_DELAY);
> +		trace_printk("rcu_reader exiting RSCS\n");
> +		rcu_read_lock();
> +		end = ktime_get();
> +		delta = ktime_to_ns(ktime_sub(end, start));
> +
> +		if (delta < 0 || (ns_to_ms(delta) > (2 * RCU_READER_DELAY))) {
> +			delta_fail = delta;
> +			test_pass = 0;
> +			break;
> +		}
> +
> +		// Don't let the rcu_reader() run more than 3s inorder to
> +		// not starve the blocker incase reader prio > blocker prio.
> +		delta = ktime_to_ns(ktime_sub(end, reader_begin));
> +		if (ns_to_ms(delta) > 3000)
> +			break;
> +	}
> +
> +	stop_test = 1;
> +	reader_exit = 1;
> +	pr_err("Exiting reader\n");
> +	return 0;
> +}
> +
> +static int rcu_blocker(void *a)
> +{
> +	int loops = 5;
> +
> +	while (!kthread_should_stop() && loops-- && !stop_test) {
> +		trace_printk("rcu_blocker entering\n");
> +		mdelay(RCU_BLOCKER_DELAY);
> +		trace_printk("rcu_blocker exiting\n");
> +	}
> +
> +	pr_err("Exiting blocker\n");
> +	stop_test = 1;
> +
> +	// Wait for reader to finish
> +	while (!reader_exit)
> +		schedule_timeout_uninterruptible(1);
> +
> +	if (test_pass)
> +		pr_err("TEST PASSED\n");
> +	else
> +		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
> +
> +	return 0;
> +}
> +
> +static int __init ptest_init(void){
> +	struct sched_param params;
> +	struct task_struct *reader, *blocker;
> +
> +	reader = kthread_create(rcu_reader, NULL, "reader");
> +	params.sched_priority = 50;
> +	sched_setscheduler(reader, SCHED_FIFO, &params);
> +	kthread_bind(reader, smp_processor_id());
> +
> +	blocker = kthread_create(rcu_blocker, NULL, "blocker");
> +	params.sched_priority = 60;
> +	sched_setscheduler(blocker, SCHED_FIFO, &params);
> +	kthread_bind(blocker, smp_processor_id());
> +
> +	wake_up_process(reader);
> +
> +	// Let reader run a little
> +	mdelay(50);
> +
> +	wake_up_process(blocker);
> +	return 0;
> +}
> +
> +static void __exit ptest_exit(void){
> +}
> +
> +module_init(ptest_init);
> +module_exit(ptest_exit);
> -- 
> 2.25.0.341.g760bfbb309-goog
> 

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Fri, Jan 17, 2020 at 05:18:04PM -0500, Joel Fernandes wrote:
> On Fri, Jan 17, 2020 at 04:58:14PM -0500, Joel Fernandes wrote:
> > Hi,
> > Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
> > test it a bit and I don't see the boost happening (thanks to Daniel for idea
> > of writing a module). Haven't debugged it more yet. Will look more tomorrow.
> > But below is the kernel module code and it prints a FAIL message to kernel
> > logs in a few seconds.
> > 
> > I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
> > is set to 500.
> > 
> > Thoughts?
> 
> So this could be because I did not start a grace period which is quite silly.
> I am sorry about that. I will add another thread to start grace periods as
> well and let you know if I still see a problem.

In addition, the RCU_READER_DELAY isn't long enough to trigger RCU priority
boosting.  And RCU_BLOCKER_DELAY would be, except that I am not seeing
an RCU read-side critical section surrounding it.

But rcutorture already has tests for RCU priority boosting.  Or are those
failing in some way?

Either way, it is good to see interest in RCU priority boosting.  ;-)

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> 
> > 
> > ---8<-----------------------
> > 
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > index c1860d35dc7e..ba34957dff26 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -2,7 +2,7 @@
> >  #
> >  # Makefile for misc devices that really don't fit anywhere else.
> >  #
> > -
> > +obj-m += ptest.o
> >  obj-$(CONFIG_IBM_ASM)		+= ibmasm/
> >  obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
> >  obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
> > diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
> > new file mode 100644
> > index 000000000000..76cc9524ccac
> > --- /dev/null
> > +++ b/drivers/misc/ptest.c
> > @@ -0,0 +1,112 @@
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/vmalloc.h>
> > +#include <linux/kthread.h>
> > +#include <linux/delay.h>
> > +
> > +#define RCU_READER_DELAY 100 //ms
> > +#define RCU_BLOCKER_DELAY 600 //ms
> > +
> > +MODULE_LICENSE("GPL");
> > +
> > +struct sched_param {
> > +	int sched_priority;
> > +};
> > +
> > +int stop_test = 0;
> > +int test_pass = 1;
> > +int reader_exit = 0;
> > +s64 delta_fail;
> > +
> > +#define ns_to_ms(delta) (delta / 1000000ULL)
> > +
> > +static int rcu_reader(void *a)
> > +{
> > +	ktime_t start, end, reader_begin;
> > +	s64 delta;
> > +
> > +	reader_begin = ktime_get();
> > +
> > +	while (!kthread_should_stop() && !stop_test) {
> > +		start = ktime_get();
> > +		rcu_read_lock();
> > +		trace_printk("rcu_reader entering RSCS\n");
> > +		mdelay(RCU_READER_DELAY);
> > +		trace_printk("rcu_reader exiting RSCS\n");
> > +		rcu_read_lock();
> > +		end = ktime_get();
> > +		delta = ktime_to_ns(ktime_sub(end, start));
> > +
> > +		if (delta < 0 || (ns_to_ms(delta) > (2 * RCU_READER_DELAY))) {
> > +			delta_fail = delta;
> > +			test_pass = 0;
> > +			break;
> > +		}
> > +
> > +		// Don't let the rcu_reader() run more than 3s inorder to
> > +		// not starve the blocker incase reader prio > blocker prio.
> > +		delta = ktime_to_ns(ktime_sub(end, reader_begin));
> > +		if (ns_to_ms(delta) > 3000)
> > +			break;
> > +	}
> > +
> > +	stop_test = 1;
> > +	reader_exit = 1;
> > +	pr_err("Exiting reader\n");
> > +	return 0;
> > +}
> > +
> > +static int rcu_blocker(void *a)
> > +{
> > +	int loops = 5;
> > +
> > +	while (!kthread_should_stop() && loops-- && !stop_test) {
> > +		trace_printk("rcu_blocker entering\n");
> > +		mdelay(RCU_BLOCKER_DELAY);
> > +		trace_printk("rcu_blocker exiting\n");
> > +	}
> > +
> > +	pr_err("Exiting blocker\n");
> > +	stop_test = 1;
> > +
> > +	// Wait for reader to finish
> > +	while (!reader_exit)
> > +		schedule_timeout_uninterruptible(1);
> > +
> > +	if (test_pass)
> > +		pr_err("TEST PASSED\n");
> > +	else
> > +		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init ptest_init(void){
> > +	struct sched_param params;
> > +	struct task_struct *reader, *blocker;
> > +
> > +	reader = kthread_create(rcu_reader, NULL, "reader");
> > +	params.sched_priority = 50;
> > +	sched_setscheduler(reader, SCHED_FIFO, &params);
> > +	kthread_bind(reader, smp_processor_id());
> > +
> > +	blocker = kthread_create(cu_blocker, NULL, "blocker");
> > +	params.sched_priority = 60;
> > +	sched_setscheduler(blocker, SCHED_FIFO, &params);
> > +	kthread_bind(blocker, smp_processor_id());
> > +
> > +	wake_up_process(reader);
> > +
> > +	// Let reader run a little
> > +	mdelay(50);
> > +
> > +	wake_up_process(blocker);
> > +	return 0;
> > +}
> > +
> > +static void __exit ptest_exit(void){
> > +}
> > +
> > +module_init(ptest_init);
> > +module_exit(ptest_exit);
> > -- 
> > 2.25.0.341.g760bfbb309-goog
> > 

Re: RCU_BOOST not working for me

[Joel Fernandes]

On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> On Fri, Jan 17, 2020 at 05:18:04PM -0500, Joel Fernandes wrote:
> > On Fri, Jan 17, 2020 at 04:58:14PM -0500, Joel Fernandes wrote:
> > > Hi,
> > > Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
> > > test it a bit and I don't see the boost happening (thanks to Daniel for idea
> > > of writing a module). Haven't debugged it more yet. Will look more tomorrow.
> > > But below is the kernel module code and it prints a FAIL message to kernel
> > > logs in a few seconds.
> > > 
> > > I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
> > > is set to 500.
> > > 
> > > Thoughts?
> > 
> > So this could be because I did not start a grace period which is quite silly.
> > I am sorry about that. I will add another thread to start grace periods as
> > well and let you know if I still see a problem.
> 
> In addition, the RCU_READER_DELAY isn't long enough to trigger RCU priority
> boosting.  And RCU_BLOCKER_DELAY would be, except that I am not seeing
> an RCU read-side critical section surrounding it.

I was assuming that only the thread being preempted needs an RCU read-side
critical section. That preempted section would inherit the priority of the
thread preempting it (the blocking thread). So the blocking thread would not
need a read-side critical section, it just would need to be higher priority
than the thread it is preempting and preempt it long enough to trigger the
boosting.

Did I miss something?

> 
> But rcutorture already has tests for RCU priority boosting.  Or are those
> failing in some way?
> 
> Either way, it is good to see interest in RCU priority boosting.  ;-)

The interest is purely academic and curiousity-driven at this point ;-).
Speaking of which why is the config not default-enabled and would it not be a
good thing to enable everywhere or there some who dislike it?

Another thought is for RCU_BOOST systems to reduce the threshold of
triggering boost dynamically if the system is at the risk of running out of
memory.

thanks,

 - Joel


> 
> 							Thanx, Paul
> 
> > thanks,
> > 
> >  - Joel
> > 
> > 
> > > 
> > > ---8<-----------------------
> > > 
> > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > > index c1860d35dc7e..ba34957dff26 100644
> > > --- a/drivers/misc/Makefile
> > > +++ b/drivers/misc/Makefile
> > > @@ -2,7 +2,7 @@
> > >  #
> > >  # Makefile for misc devices that really don't fit anywhere else.
> > >  #
> > > -
> > > +obj-m += ptest.o
> > >  obj-$(CONFIG_IBM_ASM)		+= ibmasm/
> > >  obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
> > >  obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
> > > diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
> > > new file mode 100644
> > > index 000000000000..76cc9524ccac
> > > --- /dev/null
> > > +++ b/drivers/misc/ptest.c
> > > @@ -0,0 +1,112 @@
> > > +#include <linux/init.h>
> > > +#include <linux/module.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/vmalloc.h>
> > > +#include <linux/kthread.h>
> > > +#include <linux/delay.h>
> > > +
> > > +#define RCU_READER_DELAY 100 //ms
> > > +#define RCU_BLOCKER_DELAY 600 //ms
> > > +
> > > +MODULE_LICENSE("GPL");
> > > +
> > > +struct sched_param {
> > > +	int sched_priority;
> > > +};
> > > +
> > > +int stop_test = 0;
> > > +int test_pass = 1;
> > > +int reader_exit = 0;
> > > +s64 delta_fail;
> > > +
> > > +#define ns_to_ms(delta) (delta / 1000000ULL)
> > > +
> > > +static int rcu_reader(void *a)
> > > +{
> > > +	ktime_t start, end, reader_begin;
> > > +	s64 delta;
> > > +
> > > +	reader_begin = ktime_get();
> > > +
> > > +	while (!kthread_should_stop() && !stop_test) {
> > > +		start = ktime_get();
> > > +		rcu_read_lock();
> > > +		trace_printk("rcu_reader entering RSCS\n");
> > > +		mdelay(RCU_READER_DELAY);
> > > +		trace_printk("rcu_reader exiting RSCS\n");
> > > +		rcu_read_lock();
> > > +		end = ktime_get();
> > > +		delta = ktime_to_ns(ktime_sub(end, start));
> > > +
> > > +		if (delta < 0 || (ns_to_ms(delta) > (2 * RCU_READER_DELAY))) {
> > > +			delta_fail = delta;
> > > +			test_pass = 0;
> > > +			break;
> > > +		}
> > > +
> > > +		// Don't let the rcu_reader() run more than 3s inorder to
> > > +		// not starve the blocker incase reader prio > blocker prio.
> > > +		delta = ktime_to_ns(ktime_sub(end, reader_begin));
> > > +		if (ns_to_ms(delta) > 3000)
> > > +			break;
> > > +	}
> > > +
> > > +	stop_test = 1;
> > > +	reader_exit = 1;
> > > +	pr_err("Exiting reader\n");
> > > +	return 0;
> > > +}
> > > +
> > > +static int rcu_blocker(void *a)
> > > +{
> > > +	int loops = 5;
> > > +
> > > +	while (!kthread_should_stop() && loops-- && !stop_test) {
> > > +		trace_printk("rcu_blocker entering\n");
> > > +		mdelay(RCU_BLOCKER_DELAY);
> > > +		trace_printk("rcu_blocker exiting\n");
> > > +	}
> > > +
> > > +	pr_err("Exiting blocker\n");
> > > +	stop_test = 1;
> > > +
> > > +	// Wait for reader to finish
> > > +	while (!reader_exit)
> > > +		schedule_timeout_uninterruptible(1);
> > > +
> > > +	if (test_pass)
> > > +		pr_err("TEST PASSED\n");
> > > +	else
> > > +		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int __init ptest_init(void){
> > > +	struct sched_param params;
> > > +	struct task_struct *reader, *blocker;
> > > +
> > > +	reader = kthread_create(rcu_reader, NULL, "reader");
> > > +	params.sched_priority = 50;
> > > +	sched_setscheduler(reader, SCHED_FIFO, &params);
> > > +	kthread_bind(reader, smp_processor_id());
> > > +
> > > +	blocker = kthread_create(cu_blocker, NULL, "blocker");
> > > +	params.sched_priority = 60;
> > > +	sched_setscheduler(blocker, SCHED_FIFO, &params);
> > > +	kthread_bind(blocker, smp_processor_id());
> > > +
> > > +	wake_up_process(reader);
> > > +
> > > +	// Let reader run a little
> > > +	mdelay(50);
> > > +
> > > +	wake_up_process(blocker);
> > > +	return 0;
> > > +}
> > > +
> > > +static void __exit ptest_exit(void){
> > > +}
> > > +
> > > +module_init(ptest_init);
> > > +module_exit(ptest_exit);
> > > -- 
> > > 2.25.0.341.g760bfbb309-goog
> > > 

Re: RCU_BOOST not working for me

[Joel Fernandes]

On Fri, Jan 17, 2020 at 09:10:49PM -0500, Joel Fernandes wrote:
> On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > On Fri, Jan 17, 2020 at 05:18:04PM -0500, Joel Fernandes wrote:
> > > On Fri, Jan 17, 2020 at 04:58:14PM -0500, Joel Fernandes wrote:
> > > > Hi,
> > > > Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
> > > > test it a bit and I don't see the boost happening (thanks to Daniel for idea
> > > > of writing a module). Haven't debugged it more yet. Will look more tomorrow.
> > > > But below is the kernel module code and it prints a FAIL message to kernel
> > > > logs in a few seconds.
> > > > 
> > > > I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
> > > > is set to 500.
> > > > 
> > > > Thoughts?
> > > 
> > > So this could be because I did not start a grace period which is quite silly.
> > > I am sorry about that. I will add another thread to start grace periods as
> > > well and let you know if I still see a problem.
> > 
> > In addition, the RCU_READER_DELAY isn't long enough to trigger RCU priority
> > boosting.  And RCU_BLOCKER_DELAY would be, except that I am not seeing
> > an RCU read-side critical section surrounding it.
> 
> I was assuming that only the thread being preempted needs an RCU read-side
> critical section. That preempted section would inherit the priority of the
> thread preempting it (the blocking thread). So the blocking thread would not
> need a read-side critical section, it just would need to be higher priority
> than the thread it is preempting and preempt it long enough to trigger the
> boosting.
> 
> Did I miss something?

I started GPs and still see the issue, trace as follows showing GP took 4
seconds. This time I increased RCU_BLOCK_DELAY to 800ms. Updated test patch
appended below.

          reader-240   [004] ....     5.389458: rcu_reader: rcu_reader entering RSCS
         blocker-241   [004] ....     5.443703: rcu_blocker: rcu_blocker entering
          writer-242   [004] ....     5.543743: rcu_writer: starting gp
         blocker-241   [004] ....     6.243457: rcu_blocker: rcu_blocker exiting
         blocker-241   [004] ....     6.243458: rcu_blocker: rcu_blocker entering
         blocker-241   [004] ....     7.043453: rcu_blocker: rcu_blocker exiting

	      ^^^ --- reader got blocked by blocker and did not get CPU...

         blocker-241   [004] ....     7.043453: rcu_blocker: rcu_blocker entering
         blocker-241   [004] ....     7.843450: rcu_blocker: rcu_blocker exiting
         blocker-241   [004] ....     7.843450: rcu_blocker: rcu_blocker entering
         blocker-241   [004] ....     8.643454: rcu_blocker: rcu_blocker exiting
         blocker-241   [004] ....     8.643454: rcu_blocker: rcu_blocker entering
         blocker-241   [004] ....     9.443448: rcu_blocker: rcu_blocker exiting
          reader-240   [004] ....     9.490381: rcu_reader: rcu_reader exiting RSCS

	      ^^^ --- ...until the blocker thread quit.

          writer-242   [004] ....     9.492684: rcu_writer: ending gp

---8<-----------------------

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c1860d35dc7e..ba34957dff26 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -2,7 +2,7 @@
 #
 # Makefile for misc devices that really don't fit anywhere else.
 #
-
+obj-m += ptest.o
 obj-$(CONFIG_IBM_ASM)		+= ibmasm/
 obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
 obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
new file mode 100644
index 000000000000..c7456b2f5570
--- /dev/null
+++ b/drivers/misc/ptest.c
@@ -0,0 +1,134 @@
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/vmalloc.h>
+#include <linux/kthread.h>
+#include <linux/delay.h>
+
+#define RCU_READER_DELAY 100 //ms
+#define RCU_BLOCKER_DELAY 800 //ms
+
+MODULE_LICENSE("GPL");
+
+struct sched_param {
+	int sched_priority;
+};
+
+int stop_test = 0;
+int test_pass = 1;
+int reader_exit = 0;
+s64 delta_fail;
+
+#define ns_to_ms(delta) (delta / 1000000ULL)
+
+static int rcu_writer(void *a)
+{
+	while (!kthread_should_stop() && !stop_test) {
+		trace_printk("starting gp\n");
+		synchronize_rcu();
+		trace_printk("ending gp\n");
+		msleep(10);
+	}
+
+	return 0;
+}
+
+static int rcu_reader(void *a)
+{
+	ktime_t start, end, reader_begin;
+	s64 delta;
+
+	reader_begin = ktime_get();
+
+	while (!kthread_should_stop() && !stop_test) {
+		rcu_read_lock();
+		trace_printk("rcu_reader entering RSCS\n");
+		start = ktime_get();
+		mdelay(RCU_READER_DELAY);
+		end = ktime_get();
+		trace_printk("rcu_reader exiting RSCS\n");
+		rcu_read_lock();
+		delta = ktime_to_ns(ktime_sub(end, start));
+
+		if (delta < 0 || (ns_to_ms(delta) > (2 * RCU_READER_DELAY))) {
+			delta_fail = delta;
+			test_pass = 0;
+			break;
+		}
+
+		// Don't let the rcu_reader() run more than 3s inorder to
+		// not starve the blocker incase reader prio > blocker prio.
+		delta = ktime_to_ns(ktime_sub(end, reader_begin));
+		if (ns_to_ms(delta) > 3000)
+			break;
+	}
+
+	stop_test = 1;
+	reader_exit = 1;
+	pr_err("Exiting reader\n");
+	return 0;
+}
+
+static int rcu_blocker(void *a)
+{
+	int loops = 5;
+
+	while (!kthread_should_stop() && loops-- && !stop_test) {
+		trace_printk("rcu_blocker entering\n");
+		mdelay(RCU_BLOCKER_DELAY);
+		trace_printk("rcu_blocker exiting\n");
+	}
+
+	pr_err("Exiting blocker\n");
+	stop_test = 1;
+
+	// Wait for reader to finish
+	while (!reader_exit)
+		schedule_timeout_uninterruptible(1);
+
+	if (test_pass)
+		pr_err("TEST PASSED\n");
+	else
+		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
+
+	return 0;
+}
+
+static int __init ptest_init(void){
+	struct sched_param params;
+	struct task_struct *reader, *blocker, *writer;
+
+	reader = kthread_create(rcu_reader, NULL, "reader");
+	params.sched_priority = 50;
+	sched_setscheduler(reader, SCHED_FIFO, &params);
+	kthread_bind(reader, smp_processor_id());
+
+	blocker = kthread_create(rcu_blocker, NULL, "blocker");
+	params.sched_priority = 60;
+	sched_setscheduler(blocker, SCHED_FIFO, &params);
+	kthread_bind(blocker, smp_processor_id());
+
+	writer = kthread_create(rcu_writer, NULL, "writer");
+	params.sched_priority = 70;
+	sched_setscheduler(writer, SCHED_FIFO, &params);
+	kthread_bind(writer, smp_processor_id());
+
+	wake_up_process(reader);
+
+	// Let reader run a little
+	mdelay(50);
+
+	wake_up_process(blocker);
+
+	// Let blocker run a little
+	mdelay(100);
+
+	wake_up_process(writer);
+	return 0;
+}
+
+static void __exit ptest_exit(void){
+}
+
+module_init(ptest_init);
+module_exit(ptest_exit);
-- 
2.25.0.341.g760bfbb309-goog

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Fri, Jan 17, 2020 at 09:32:22PM -0500, Joel Fernandes wrote:
> On Fri, Jan 17, 2020 at 09:10:49PM -0500, Joel Fernandes wrote:
> > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > > On Fri, Jan 17, 2020 at 05:18:04PM -0500, Joel Fernandes wrote:
> > > > On Fri, Jan 17, 2020 at 04:58:14PM -0500, Joel Fernandes wrote:
> > > > > Hi,
> > > > > Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
> > > > > test it a bit and I don't see the boost happening (thanks to Daniel for idea
> > > > > of writing a module). Haven't debugged it more yet. Will look more tomorrow.
> > > > > But below is the kernel module code and it prints a FAIL message to kernel
> > > > > logs in a few seconds.
> > > > > 
> > > > > I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
> > > > > is set to 500.
> > > > > 
> > > > > Thoughts?
> > > > 
> > > > So this could be because I did not start a grace period which is quite silly.
> > > > I am sorry about that. I will add another thread to start grace periods as
> > > > well and let you know if I still see a problem.
> > > 
> > > In addition, the RCU_READER_DELAY isn't long enough to trigger RCU priority
> > > boosting.  And RCU_BLOCKER_DELAY would be, except that I am not seeing
> > > an RCU read-side critical section surrounding it.
> > 
> > I was assuming that only the thread being preempted needs an RCU read-side
> > critical section. That preempted section would inherit the priority of the
> > thread preempting it (the blocking thread). So the blocking thread would not
> > need a read-side critical section, it just would need to be higher priority
> > than the thread it is preempting and preempt it long enough to trigger the
> > boosting.
> > 
> > Did I miss something?
> 
> I started GPs and still see the issue, trace as follows showing GP took 4
> seconds. This time I increased RCU_BLOCK_DELAY to 800ms. Updated test patch
> appended below.

Did you boot with kernel parameter rcu_tree.kthread_prio=61 or more?  If not,
RCU priority boosting is not going to be able to help.

The writer does nothing for you.  The priority of the task invoking
synchronize_rcu() is irrelevant because synchronize_rcu() is inherently
non-realtime.

							Thanx, Paul

>           reader-240   [004] ....     5.389458: rcu_reader: rcu_reader entering RSCS
>          blocker-241   [004] ....     5.443703: rcu_blocker: rcu_blocker entering
>           writer-242   [004] ....     5.543743: rcu_writer: starting gp
>          blocker-241   [004] ....     6.243457: rcu_blocker: rcu_blocker exiting
>          blocker-241   [004] ....     6.243458: rcu_blocker: rcu_blocker entering
>          blocker-241   [004] ....     7.043453: rcu_blocker: rcu_blocker exiting
> 
> 	      ^^^ --- reader got blocked by blocker and did not get CPU...
> 
>          blocker-241   [004] ....     7.043453: rcu_blocker: rcu_blocker entering
>          blocker-241   [004] ....     7.843450: rcu_blocker: rcu_blocker exiting
>          blocker-241   [004] ....     7.843450: rcu_blocker: rcu_blocker entering
>          blocker-241   [004] ....     8.643454: rcu_blocker: rcu_blocker exiting
>          blocker-241   [004] ....     8.643454: rcu_blocker: rcu_blocker entering
>          blocker-241   [004] ....     9.443448: rcu_blocker: rcu_blocker exiting
>           reader-240   [004] ....     9.490381: rcu_reader: rcu_reader exiting RSCS
> 
> 	      ^^^ --- ...until the blocker thread quit.
> 
>           writer-242   [004] ....     9.492684: rcu_writer: ending gp
> 
> ---8<-----------------------
> 
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index c1860d35dc7e..ba34957dff26 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -2,7 +2,7 @@
>  #
>  # Makefile for misc devices that really don't fit anywhere else.
>  #
> -
> +obj-m += ptest.o
>  obj-$(CONFIG_IBM_ASM)		+= ibmasm/
>  obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
>  obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
> diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
> new file mode 100644
> index 000000000000..c7456b2f5570
> --- /dev/null
> +++ b/drivers/misc/ptest.c
> @@ -0,0 +1,134 @@
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/vmalloc.h>
> +#include <linux/kthread.h>
> +#include <linux/delay.h>
> +
> +#define RCU_READER_DELAY 100 //ms
> +#define RCU_BLOCKER_DELAY 800 //ms
> +
> +MODULE_LICENSE("GPL");
> +
> +struct sched_param {
> +	int sched_priority;
> +};
> +
> +int stop_test = 0;
> +int test_pass = 1;
> +int reader_exit = 0;
> +s64 delta_fail;
> +
> +#define ns_to_ms(delta) (delta / 1000000ULL)
> +
> +static int rcu_writer(void *a)
> +{
> +	while (!kthread_should_stop() && !stop_test) {
> +		trace_printk("starting gp\n");
> +		synchronize_rcu();
> +		trace_printk("ending gp\n");
> +		msleep(10);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rcu_reader(void *a)
> +{
> +	ktime_t start, end, reader_begin;
> +	s64 delta;
> +
> +	reader_begin = ktime_get();
> +
> +	while (!kthread_should_stop() && !stop_test) {
> +		rcu_read_lock();
> +		trace_printk("rcu_reader entering RSCS\n");
> +		start = ktime_get();
> +		mdelay(RCU_READER_DELAY);
> +		end = ktime_get();
> +		trace_printk("rcu_reader exiting RSCS\n");
> +		rcu_read_lock();
> +		delta = ktime_to_ns(ktime_sub(end, start));
> +
> +		if (delta < 0 || (ns_to_ms(delta) > (2 * RCU_READER_DELAY))) {
> +			delta_fail = delta;
> +			test_pass = 0;
> +			break;
> +		}
> +
> +		// Don't let the rcu_reader() run more than 3s inorder to
> +		// not starve the blocker incase reader prio > blocker prio.
> +		delta = ktime_to_ns(ktime_sub(end, reader_begin));
> +		if (ns_to_ms(delta) > 3000)
> +			break;
> +	}
> +
> +	stop_test = 1;
> +	reader_exit = 1;
> +	pr_err("Exiting reader\n");
> +	return 0;
> +}
> +
> +static int rcu_blocker(void *a)
> +{
> +	int loops = 5;
> +
> +	while (!kthread_should_stop() && loops-- && !stop_test) {
> +		trace_printk("rcu_blocker entering\n");
> +		mdelay(RCU_BLOCKER_DELAY);
> +		trace_printk("rcu_blocker exiting\n");
> +	}
> +
> +	pr_err("Exiting blocker\n");
> +	stop_test = 1;
> +
> +	// Wait for reader to finish
> +	while (!reader_exit)
> +		schedule_timeout_uninterruptible(1);
> +
> +	if (test_pass)
> +		pr_err("TEST PASSED\n");
> +	else
> +		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
> +
> +	return 0;
> +}
> +
> +static int __init ptest_init(void){
> +	struct sched_param params;
> +	struct task_struct *reader, *blocker, *writer;
> +
> +	reader = kthread_create(rcu_reader, NULL, "reader");
> +	params.sched_priority = 50;
> +	sched_setscheduler(reader, SCHED_FIFO, &params);
> +	kthread_bind(reader, smp_processor_id());
> +
> +	blocker = kthread_create(rcu_blocker, NULL, "blocker");
> +	params.sched_priority = 60;
> +	sched_setscheduler(blocker, SCHED_FIFO, &params);
> +	kthread_bind(blocker, smp_processor_id());
> +
> +	writer = kthread_create(rcu_writer, NULL, "writer");
> +	params.sched_priority = 70;
> +	sched_setscheduler(writer, SCHED_FIFO, &params);
> +	kthread_bind(writer, smp_processor_id());
> +
> +	wake_up_process(reader);
> +
> +	// Let reader run a little
> +	mdelay(50);
> +
> +	wake_up_process(blocker);
> +
> +	// Let blocker run a little
> +	mdelay(100);
> +
> +	wake_up_process(writer);
> +	return 0;
> +}
> +
> +static void __exit ptest_exit(void){
> +}
> +
> +module_init(ptest_init);
> +module_exit(ptest_exit);
> -- 
> 2.25.0.341.g760bfbb309-goog
> 

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Fri, Jan 17, 2020 at 09:10:49PM -0500, Joel Fernandes wrote:
> On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > On Fri, Jan 17, 2020 at 05:18:04PM -0500, Joel Fernandes wrote:
> > > On Fri, Jan 17, 2020 at 04:58:14PM -0500, Joel Fernandes wrote:
> > > > Hi,
> > > > Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
> > > > test it a bit and I don't see the boost happening (thanks to Daniel for idea
> > > > of writing a module). Haven't debugged it more yet. Will look more tomorrow.
> > > > But below is the kernel module code and it prints a FAIL message to kernel
> > > > logs in a few seconds.
> > > > 
> > > > I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
> > > > is set to 500.
> > > > 
> > > > Thoughts?
> > > 
> > > So this could be because I did not start a grace period which is quite silly.
> > > I am sorry about that. I will add another thread to start grace periods as
> > > well and let you know if I still see a problem.
> > 
> > In addition, the RCU_READER_DELAY isn't long enough to trigger RCU priority
> > boosting.  And RCU_BLOCKER_DELAY would be, except that I am not seeing
> > an RCU read-side critical section surrounding it.
> 
> I was assuming that only the thread being preempted needs an RCU read-side
> critical section. That preempted section would inherit the priority of the
> thread preempting it (the blocking thread). So the blocking thread would not
> need a read-side critical section, it just would need to be higher priority
> than the thread it is preempting and preempt it long enough to trigger the
> boosting.
> 
> Did I miss something?

Yes.  That is not how RCU priority boosting works.

What happens instead is that a set of rcub kthreads (one per leaf
rcu_node structure) run at the SCHED_FIFO priority specified by the
rcutree.kthread_prio kernel-boot parameter (as does the grace-period
kthread when RCU priority boosting is enabled).  When the grace-period
kthread decides that boosting is needed, it awakens the relevant rcub
kthread.  The rcub kthread initializes an rt_mutex into a state where
it appears to be held by the task that has been too long in an RCU
read-side critical section, then acquires the lock.  The lock-based
priority-boosting mechanism kicks in at that point.  (I heard this
approach from tglx.)

And I missed something as well, namely that everything is bound to the
same CPU in your test.  But did you remember to set rcutree.kthread_prio
to greater than 60?  It defaults to 1 when RCU priority boosting is
enabled, which isn't going to do much to a prio-50 SCHED_FIFO task,
let alone a prio-60 SCHED_FIFO task.

> > But rcutorture already has tests for RCU priority boosting.  Or are those
> > failing in some way?
> > 
> > Either way, it is good to see interest in RCU priority boosting.  ;-)
> 
> The interest is purely academic and curiousity-driven at this point ;-).

Fair enough!

> Speaking of which why is the config not default-enabled and would it not be a
> good thing to enable everywhere or there some who dislike it?

It is enabled by default in the -rt kernel.  It has been some time since
I proposed enabling it by default in mainline, but the last time that
proposal was not well received.  My approach is to wait until it becomes
a problem in mainline, and if it does, propose making it the default in
mainline.

Except that I haven't heard of any such problems, so I have made no
such proposal.

> Another thought is for RCU_BOOST systems to reduce the threshold of
> triggering boost dynamically if the system is at the risk of running out of
> memory.

Agreed, and that is in fact the purpose of the check of rcu_state.cbovld
in rcu_initiate_boost().  And why we need to get the number of
outstanding kfree_rcu() blocks fed into the computation leading up to
rcu_state.cbovld.  ;-)

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> 
> > 
> > 							Thanx, Paul
> > 
> > > thanks,
> > > 
> > >  - Joel
> > > 
> > > 
> > > > 
> > > > ---8<-----------------------
> > > > 
> > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > > > index c1860d35dc7e..ba34957dff26 100644
> > > > --- a/drivers/misc/Makefile
> > > > +++ b/drivers/misc/Makefile
> > > > @@ -2,7 +2,7 @@
> > > >  #
> > > >  # Makefile for misc devices that really don't fit anywhere else.
> > > >  #
> > > > -
> > > > +obj-m += ptest.o
> > > >  obj-$(CONFIG_IBM_ASM)		+= ibmasm/
> > > >  obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
> > > >  obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
> > > > diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
> > > > new file mode 100644
> > > > index 000000000000..76cc9524ccac
> > > > --- /dev/null
> > > > +++ b/drivers/misc/ptest.c
> > > > @@ -0,0 +1,112 @@
> > > > +#include <linux/init.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/vmalloc.h>
> > > > +#include <linux/kthread.h>
> > > > +#include <linux/delay.h>
> > > > +
> > > > +#define RCU_READER_DELAY 100 //ms
> > > > +#define RCU_BLOCKER_DELAY 600 //ms
> > > > +
> > > > +MODULE_LICENSE("GPL");
> > > > +
> > > > +struct sched_param {
> > > > +	int sched_priority;
> > > > +};
> > > > +
> > > > +int stop_test = 0;
> > > > +int test_pass = 1;
> > > > +int reader_exit = 0;
> > > > +s64 delta_fail;
> > > > +
> > > > +#define ns_to_ms(delta) (delta / 1000000ULL)
> > > > +
> > > > +static int rcu_reader(void *a)
> > > > +{
> > > > +	ktime_t start, end, reader_begin;
> > > > +	s64 delta;
> > > > +
> > > > +	reader_begin = ktime_get();
> > > > +
> > > > +	while (!kthread_should_stop() && !stop_test) {
> > > > +		start = ktime_get();
> > > > +		rcu_read_lock();
> > > > +		trace_printk("rcu_reader entering RSCS\n");
> > > > +		mdelay(RCU_READER_DELAY);
> > > > +		trace_printk("rcu_reader exiting RSCS\n");
> > > > +		rcu_read_lock();
> > > > +		end = ktime_get();
> > > > +		delta = ktime_to_ns(ktime_sub(end, start));
> > > > +
> > > > +		if (delta < 0 || (ns_to_ms(delta) > (2 * RCU_READER_DELAY))) {
> > > > +			delta_fail = delta;
> > > > +			test_pass = 0;
> > > > +			break;
> > > > +		}
> > > > +
> > > > +		// Don't let the rcu_reader() run more than 3s inorder to
> > > > +		// not starve the blocker incase reader prio > blocker prio.
> > > > +		delta = ktime_to_ns(ktime_sub(end, reader_begin));
> > > > +		if (ns_to_ms(delta) > 3000)
> > > > +			break;
> > > > +	}
> > > > +
> > > > +	stop_test = 1;
> > > > +	reader_exit = 1;
> > > > +	pr_err("Exiting reader\n");
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int rcu_blocker(void *a)
> > > > +{
> > > > +	int loops = 5;
> > > > +
> > > > +	while (!kthread_should_stop() && loops-- && !stop_test) {
> > > > +		trace_printk("rcu_blocker entering\n");
> > > > +		mdelay(RCU_BLOCKER_DELAY);
> > > > +		trace_printk("rcu_blocker exiting\n");
> > > > +	}
> > > > +
> > > > +	pr_err("Exiting blocker\n");
> > > > +	stop_test = 1;
> > > > +
> > > > +	// Wait for reader to finish
> > > > +	while (!reader_exit)
> > > > +		schedule_timeout_uninterruptible(1);
> > > > +
> > > > +	if (test_pass)
> > > > +		pr_err("TEST PASSED\n");
> > > > +	else
> > > > +		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int __init ptest_init(void){
> > > > +	struct sched_param params;
> > > > +	struct task_struct *reader, *blocker;
> > > > +
> > > > +	reader = kthread_create(rcu_reader, NULL, "reader");
> > > > +	params.sched_priority = 50;
> > > > +	sched_setscheduler(reader, SCHED_FIFO, &params);
> > > > +	kthread_bind(reader, smp_processor_id());
> > > > +
> > > > +	blocker = kthread_create(cu_blocker, NULL, "blocker");
> > > > +	params.sched_priority = 60;
> > > > +	sched_setscheduler(blocker, SCHED_FIFO, &params);
> > > > +	kthread_bind(blocker, smp_processor_id());
> > > > +
> > > > +	wake_up_process(reader);
> > > > +
> > > > +	// Let reader run a little
> > > > +	mdelay(50);
> > > > +
> > > > +	wake_up_process(blocker);
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static void __exit ptest_exit(void){
> > > > +}
> > > > +
> > > > +module_init(ptest_init);
> > > > +module_exit(ptest_exit);
> > > > -- 
> > > > 2.25.0.341.g760bfbb309-goog
> > > > 

Re: RCU_BOOST not working for me

[Joel Fernandes]

On Fri, Jan 17, 2020 at 08:28:26PM -0800, Paul E. McKenney wrote:
> On Fri, Jan 17, 2020 at 09:10:49PM -0500, Joel Fernandes wrote:
> > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > > On Fri, Jan 17, 2020 at 05:18:04PM -0500, Joel Fernandes wrote:
> > > > On Fri, Jan 17, 2020 at 04:58:14PM -0500, Joel Fernandes wrote:
> > > > > Hi,
> > > > > Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
> > > > > test it a bit and I don't see the boost happening (thanks to Daniel for idea
> > > > > of writing a module). Haven't debugged it more yet. Will look more tomorrow.
> > > > > But below is the kernel module code and it prints a FAIL message to kernel
> > > > > logs in a few seconds.
> > > > > 
> > > > > I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
> > > > > is set to 500.
> > > > > 
> > > > > Thoughts?
> > > > 
> > > > So this could be because I did not start a grace period which is quite silly.
> > > > I am sorry about that. I will add another thread to start grace periods as
> > > > well and let you know if I still see a problem.
> > > 
> > > In addition, the RCU_READER_DELAY isn't long enough to trigger RCU priority
> > > boosting.  And RCU_BLOCKER_DELAY would be, except that I am not seeing
> > > an RCU read-side critical section surrounding it.
> > 
> > I was assuming that only the thread being preempted needs an RCU read-side
> > critical section. That preempted section would inherit the priority of the
> > thread preempting it (the blocking thread). So the blocking thread would not
> > need a read-side critical section, it just would need to be higher priority
> > than the thread it is preempting and preempt it long enough to trigger the
> > boosting.
> > 
> > Did I miss something?
> 
> Yes.  That is not how RCU priority boosting works.
> 
> What happens instead is that a set of rcub kthreads (one per leaf
> rcu_node structure) run at the SCHED_FIFO priority specified by the
> rcutree.kthread_prio kernel-boot parameter (as does the grace-period
> kthread when RCU priority boosting is enabled).  When the grace-period
> kthread decides that boosting is needed, it awakens the relevant rcub
> kthread.  The rcub kthread initializes an rt_mutex into a state where
> it appears to be held by the task that has been too long in an RCU
> read-side critical section, then acquires the lock.  The lock-based
> priority-boosting mechanism kicks in at that point.  (I heard this
> approach from tglx.)

Thanks for the details. It makes sense to me.

> And I missed something as well, namely that everything is bound to the
> same CPU in your test.  But did you remember to set rcutree.kthread_prio
> to greater than 60?  It defaults to 1 when RCU priority boosting is
> enabled, which isn't going to do much to a prio-50 SCHED_FIFO task,
> let alone a prio-60 SCHED_FIFO task.

Yes indeed this was the issue. I set rcutree.kthread_prio to 90 and see the
test passing now. Below is the updated test code for archival.

But wouldn't this be an issue unless rcu.kthread_prio is set the MAX_RT_PRIO
or some high number? Because otherwise there could always be threads that
don't get boosted.

> > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > failing in some way?
> > > 
> > > Either way, it is good to see interest in RCU priority boosting.  ;-)
> > 
> > The interest is purely academic and curiousity-driven at this point ;-).
> 
> Fair enough!
> 
> > Speaking of which why is the config not default-enabled and would it not be a
> > good thing to enable everywhere or there some who dislike it?
> 
> It is enabled by default in the -rt kernel.  It has been some time since
> I proposed enabling it by default in mainline, but the last time that
> proposal was not well received.  My approach is to wait until it becomes
> a problem in mainline, and if it does, propose making it the default in
> mainline.

It could also be some problem that no one has root caused to a lack of
boosting?  ;-)

> Except that I haven't heard of any such problems, so I have made no
> such proposal.

Makes sense.

> > Another thought is for RCU_BOOST systems to reduce the threshold of
> > triggering boost dynamically if the system is at the risk of running out of
> > memory.
> 
> Agreed, and that is in fact the purpose of the check of rcu_state.cbovld
> in rcu_initiate_boost().  And why we need to get the number of
> outstanding kfree_rcu() blocks fed into the computation leading up to
> rcu_state.cbovld.  ;-)

Oh, I see this new code. I will study it and look into it more. Agreed on the
feeding of kfree_rcu() blocks and/or memory pressure information into the
computations leading up to cbovld.

thanks,

 - Joel

---8<-----------------------

From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Subject: [PATCH] Kernel module to test RCU_BOOST

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 drivers/misc/Makefile |   2 +-
 drivers/misc/ptest.c  | 141 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 drivers/misc/ptest.c

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c1860d35dc7e..ba34957dff26 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -2,7 +2,7 @@
 #
 # Makefile for misc devices that really don't fit anywhere else.
 #
-
+obj-m += ptest.o
 obj-$(CONFIG_IBM_ASM)		+= ibmasm/
 obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
 obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
new file mode 100644
index 000000000000..e5ece58e45ea
--- /dev/null
+++ b/drivers/misc/ptest.c
@@ -0,0 +1,141 @@
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/vmalloc.h>
+#include <linux/kthread.h>
+#include <linux/delay.h>
+
+#define RCU_READER_DELAY 100 //ms
+#define RCU_BLOCKER_DELAY 800 //ms
+
+/* Max delta in rcu_reader beyond which we can
+ * say boosting failed.  CONFIG_RCU_BOOST=200 in my setup plus booster wakes up
+ * after 50ms and writer wakes up 150 ms after that So the GP starts only 150ms
+ * later. To be safe, give a max of 400ms for the reader-section.
+ */
+#define RCU_READER_MAX_DELTA 400 // ms
+
+MODULE_LICENSE("GPL");
+
+struct sched_param {
+	int sched_priority;
+};
+
+int stop_test = 0;
+int test_pass = 1;
+int reader_exit = 0;
+s64 delta_fail;
+
+#define ns_to_ms(delta) (delta / 1000000ULL)
+
+static int rcu_writer(void *a)
+{
+	while (!kthread_should_stop() && !stop_test) {
+		trace_printk("starting gp\n");
+		synchronize_rcu();
+		trace_printk("ending gp\n");
+		msleep(10);
+	}
+
+	return 0;
+}
+
+static int rcu_reader(void *a)
+{
+	ktime_t start, end, reader_begin;
+	s64 delta;
+
+	reader_begin = ktime_get();
+
+	while (!kthread_should_stop() && !stop_test) {
+		rcu_read_lock();
+		trace_printk("rcu_reader entering RSCS\n");
+		start = ktime_get();
+		mdelay(RCU_READER_DELAY);
+		end = ktime_get();
+		trace_printk("rcu_reader exiting RSCS\n");
+		rcu_read_lock();
+		delta = ktime_to_ns(ktime_sub(end, start));
+
+		if (delta < 0 || (ns_to_ms(delta) > RCU_READER_MAX_DELTA)) {
+			delta_fail = delta;
+			test_pass = 0;
+			break;
+		}
+
+		// Don't let the rcu_reader() run more than 3s inorder to
+		// not starve the blocker incase reader prio > blocker prio.
+		delta = ktime_to_ns(ktime_sub(end, reader_begin));
+		if (ns_to_ms(delta) > 3000)
+			break;
+	}
+
+	stop_test = 1;
+	reader_exit = 1;
+	pr_err("Exiting reader\n");
+	return 0;
+}
+
+static int rcu_blocker(void *a)
+{
+	int loops = 5;
+
+	while (!kthread_should_stop() && loops-- && !stop_test) {
+		trace_printk("rcu_blocker entering\n");
+		mdelay(RCU_BLOCKER_DELAY);
+		trace_printk("rcu_blocker exiting\n");
+	}
+
+	pr_err("Exiting blocker\n");
+	stop_test = 1;
+
+	// Wait for reader to finish
+	while (!reader_exit)
+		schedule_timeout_uninterruptible(1);
+
+	if (test_pass)
+		pr_err("TEST PASSED\n");
+	else
+		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
+
+	return 0;
+}
+
+static int __init ptest_init(void){
+	struct sched_param params;
+	struct task_struct *reader, *blocker, *writer;
+
+	reader = kthread_create(rcu_reader, NULL, "reader");
+	params.sched_priority = 50;
+	sched_setscheduler(reader, SCHED_FIFO, &params);
+	kthread_bind(reader, smp_processor_id());
+
+	blocker = kthread_create(rcu_blocker, NULL, "blocker");
+	params.sched_priority = 60;
+	sched_setscheduler(blocker, SCHED_FIFO, &params);
+	kthread_bind(blocker, smp_processor_id());
+
+	writer = kthread_create(rcu_writer, NULL, "writer");
+	params.sched_priority = 70;
+	sched_setscheduler(writer, SCHED_FIFO, &params);
+	kthread_bind(writer, smp_processor_id());
+
+	wake_up_process(reader);
+
+	// Let reader run a little
+	mdelay(50);
+
+	wake_up_process(blocker);
+
+	// Let blocker run a little
+	mdelay(100);
+
+	wake_up_process(writer);
+	return 0;
+}
+
+static void __exit ptest_exit(void){
+}
+
+module_init(ptest_init);
+module_exit(ptest_exit);
-- 
2.25.0.341.g760bfbb309-goog

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Sat, Jan 18, 2020 at 03:12:08PM -0500, Joel Fernandes wrote:
> On Fri, Jan 17, 2020 at 08:28:26PM -0800, Paul E. McKenney wrote:
> > On Fri, Jan 17, 2020 at 09:10:49PM -0500, Joel Fernandes wrote:
> > > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > > > On Fri, Jan 17, 2020 at 05:18:04PM -0500, Joel Fernandes wrote:
> > > > > On Fri, Jan 17, 2020 at 04:58:14PM -0500, Joel Fernandes wrote:
> > > > > > Hi,
> > > > > > Me and Daniel were poking around with RCU_BOOST. I wrote a kernel module to
> > > > > > test it a bit and I don't see the boost happening (thanks to Daniel for idea
> > > > > > of writing a module). Haven't debugged it more yet. Will look more tomorrow.
> > > > > > But below is the kernel module code and it prints a FAIL message to kernel
> > > > > > logs in a few seconds.
> > > > > > 
> > > > > > I see the reader thread not getting CPU for several seconds. RCU_BOOST_DELAY
> > > > > > is set to 500.
> > > > > > 
> > > > > > Thoughts?
> > > > > 
> > > > > So this could be because I did not start a grace period which is quite silly.
> > > > > I am sorry about that. I will add another thread to start grace periods as
> > > > > well and let you know if I still see a problem.
> > > > 
> > > > In addition, the RCU_READER_DELAY isn't long enough to trigger RCU priority
> > > > boosting.  And RCU_BLOCKER_DELAY would be, except that I am not seeing
> > > > an RCU read-side critical section surrounding it.
> > > 
> > > I was assuming that only the thread being preempted needs an RCU read-side
> > > critical section. That preempted section would inherit the priority of the
> > > thread preempting it (the blocking thread). So the blocking thread would not
> > > need a read-side critical section, it just would need to be higher priority
> > > than the thread it is preempting and preempt it long enough to trigger the
> > > boosting.
> > > 
> > > Did I miss something?
> > 
> > Yes.  That is not how RCU priority boosting works.
> > 
> > What happens instead is that a set of rcub kthreads (one per leaf
> > rcu_node structure) run at the SCHED_FIFO priority specified by the
> > rcutree.kthread_prio kernel-boot parameter (as does the grace-period
> > kthread when RCU priority boosting is enabled).  When the grace-period
> > kthread decides that boosting is needed, it awakens the relevant rcub
> > kthread.  The rcub kthread initializes an rt_mutex into a state where
> > it appears to be held by the task that has been too long in an RCU
> > read-side critical section, then acquires the lock.  The lock-based
> > priority-boosting mechanism kicks in at that point.  (I heard this
> > approach from tglx.)
> 
> Thanks for the details. It makes sense to me.

Glad it helped!

> > And I missed something as well, namely that everything is bound to the
> > same CPU in your test.  But did you remember to set rcutree.kthread_prio
> > to greater than 60?  It defaults to 1 when RCU priority boosting is
> > enabled, which isn't going to do much to a prio-50 SCHED_FIFO task,
> > let alone a prio-60 SCHED_FIFO task.
> 
> Yes indeed this was the issue. I set rcutree.kthread_prio to 90 and see the
> test passing now. Below is the updated test code for archival.

Very good!

> But wouldn't this be an issue unless rcu.kthread_prio is set the MAX_RT_PRIO
> or some high number? Because otherwise there could always be threads that
> don't get boosted.

It depends on the details of the real-time application and system.

Some systems might need rcu.kthread_prio to be set to (say) 57 so that it
didn't interfere with critical userspace tasks running at 58 and above.
So why default to 1 for CONFIG_RCU_BOOST in mainline?  Because the main
usecase there is a system with a massive overload of SCHED_NORMAL tasks.
In this case, SCHED_FIFO priority 1 suffices and minimally interferes
with any realtime workload that might be running.

> > > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > > failing in some way?
> > > > 
> > > > Either way, it is good to see interest in RCU priority boosting.  ;-)
> > > 
> > > The interest is purely academic and curiousity-driven at this point ;-).
> > 
> > Fair enough!
> > 
> > > Speaking of which why is the config not default-enabled and would it not be a
> > > good thing to enable everywhere or there some who dislike it?
> > 
> > It is enabled by default in the -rt kernel.  It has been some time since
> > I proposed enabling it by default in mainline, but the last time that
> > proposal was not well received.  My approach is to wait until it becomes
> > a problem in mainline, and if it does, propose making it the default in
> > mainline.
> 
> It could also be some problem that no one has root caused to a lack of
> boosting?  ;-)

Quite possibly.  But please understand that setting rcutree.kthread_prio
even to 1 significantly increasees context-switch overhead, which is why
it defaults to zero for CONFIG_RCU_BOOST=n and is also another reason
why CONFIG_RCU_BOOST defaults to n.

> > Except that I haven't heard of any such problems, so I have made no
> > such proposal.
> 
> Makes sense.
> 
> > > Another thought is for RCU_BOOST systems to reduce the threshold of
> > > triggering boost dynamically if the system is at the risk of running out of
> > > memory.
> > 
> > Agreed, and that is in fact the purpose of the check of rcu_state.cbovld
> > in rcu_initiate_boost().  And why we need to get the number of
> > outstanding kfree_rcu() blocks fed into the computation leading up to
> > rcu_state.cbovld.  ;-)
> 
> Oh, I see this new code. I will study it and look into it more. Agreed on the
> feeding of kfree_rcu() blocks and/or memory pressure information into the
> computations leading up to cbovld.

Very good!

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> ---8<-----------------------
> 
> From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> Subject: [PATCH] Kernel module to test RCU_BOOST
> 
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
>  drivers/misc/Makefile |   2 +-
>  drivers/misc/ptest.c  | 141 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 142 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/misc/ptest.c
> 
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index c1860d35dc7e..ba34957dff26 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -2,7 +2,7 @@
>  #
>  # Makefile for misc devices that really don't fit anywhere else.
>  #
> -
> +obj-m += ptest.o
>  obj-$(CONFIG_IBM_ASM)		+= ibmasm/
>  obj-$(CONFIG_IBMVMC)		+= ibmvmc.o
>  obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
> diff --git a/drivers/misc/ptest.c b/drivers/misc/ptest.c
> new file mode 100644
> index 000000000000..e5ece58e45ea
> --- /dev/null
> +++ b/drivers/misc/ptest.c
> @@ -0,0 +1,141 @@
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/vmalloc.h>
> +#include <linux/kthread.h>
> +#include <linux/delay.h>
> +
> +#define RCU_READER_DELAY 100 //ms
> +#define RCU_BLOCKER_DELAY 800 //ms
> +
> +/* Max delta in rcu_reader beyond which we can
> + * say boosting failed.  CONFIG_RCU_BOOST=200 in my setup plus booster wakes up
> + * after 50ms and writer wakes up 150 ms after that So the GP starts only 150ms
> + * later. To be safe, give a max of 400ms for the reader-section.
> + */
> +#define RCU_READER_MAX_DELTA 400 // ms
> +
> +MODULE_LICENSE("GPL");
> +
> +struct sched_param {
> +	int sched_priority;
> +};
> +
> +int stop_test = 0;
> +int test_pass = 1;
> +int reader_exit = 0;
> +s64 delta_fail;
> +
> +#define ns_to_ms(delta) (delta / 1000000ULL)
> +
> +static int rcu_writer(void *a)
> +{
> +	while (!kthread_should_stop() && !stop_test) {
> +		trace_printk("starting gp\n");
> +		synchronize_rcu();
> +		trace_printk("ending gp\n");
> +		msleep(10);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rcu_reader(void *a)
> +{
> +	ktime_t start, end, reader_begin;
> +	s64 delta;
> +
> +	reader_begin = ktime_get();
> +
> +	while (!kthread_should_stop() && !stop_test) {
> +		rcu_read_lock();
> +		trace_printk("rcu_reader entering RSCS\n");
> +		start = ktime_get();
> +		mdelay(RCU_READER_DELAY);
> +		end = ktime_get();
> +		trace_printk("rcu_reader exiting RSCS\n");
> +		rcu_read_lock();
> +		delta = ktime_to_ns(ktime_sub(end, start));
> +
> +		if (delta < 0 || (ns_to_ms(delta) > RCU_READER_MAX_DELTA)) {
> +			delta_fail = delta;
> +			test_pass = 0;
> +			break;
> +		}
> +
> +		// Don't let the rcu_reader() run more than 3s inorder to
> +		// not starve the blocker incase reader prio > blocker prio.
> +		delta = ktime_to_ns(ktime_sub(end, reader_begin));
> +		if (ns_to_ms(delta) > 3000)
> +			break;
> +	}
> +
> +	stop_test = 1;
> +	reader_exit = 1;
> +	pr_err("Exiting reader\n");
> +	return 0;
> +}
> +
> +static int rcu_blocker(void *a)
> +{
> +	int loops = 5;
> +
> +	while (!kthread_should_stop() && loops-- && !stop_test) {
> +		trace_printk("rcu_blocker entering\n");
> +		mdelay(RCU_BLOCKER_DELAY);
> +		trace_printk("rcu_blocker exiting\n");
> +	}
> +
> +	pr_err("Exiting blocker\n");
> +	stop_test = 1;
> +
> +	// Wait for reader to finish
> +	while (!reader_exit)
> +		schedule_timeout_uninterruptible(1);
> +
> +	if (test_pass)
> +		pr_err("TEST PASSED\n");
> +	else
> +		pr_err("TEST FAILED, failing delta=%lldms\n", ns_to_ms(delta_fail));
> +
> +	return 0;
> +}
> +
> +static int __init ptest_init(void){
> +	struct sched_param params;
> +	struct task_struct *reader, *blocker, *writer;
> +
> +	reader = kthread_create(rcu_reader, NULL, "reader");
> +	params.sched_priority = 50;
> +	sched_setscheduler(reader, SCHED_FIFO, &params);
> +	kthread_bind(reader, smp_processor_id());
> +
> +	blocker = kthread_create(rcu_blocker, NULL, "blocker");
> +	params.sched_priority = 60;
> +	sched_setscheduler(blocker, SCHED_FIFO, &params);
> +	kthread_bind(blocker, smp_processor_id());
> +
> +	writer = kthread_create(rcu_writer, NULL, "writer");
> +	params.sched_priority = 70;
> +	sched_setscheduler(writer, SCHED_FIFO, &params);
> +	kthread_bind(writer, smp_processor_id());
> +
> +	wake_up_process(reader);
> +
> +	// Let reader run a little
> +	mdelay(50);
> +
> +	wake_up_process(blocker);
> +
> +	// Let blocker run a little
> +	mdelay(100);
> +
> +	wake_up_process(writer);
> +	return 0;
> +}
> +
> +static void __exit ptest_exit(void){
> +}
> +
> +module_init(ptest_init);
> +module_exit(ptest_exit);
> -- 
> 2.25.0.341.g760bfbb309-goog
> 

Re: RCU_BOOST not working for me

[Joel Fernandes]

On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
[...] 
> But rcutorture already has tests for RCU priority boosting.  Or are those
> failing in some way?

Yes there are tests, but I thought of just a simple experiment to study this.
Purely since it is existing RCU kernel code that I'd like to understand. And
me/Daniel are also looking into possibly using run-time / trace-based
verification some of these behaviors.

thanks,

 - Joel

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Fri, Jan 17, 2020 at 09:34:34PM -0500, Joel Fernandes wrote:
> On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> [...] 
> > But rcutorture already has tests for RCU priority boosting.  Or are those
> > failing in some way?
> 
> Yes there are tests, but I thought of just a simple experiment to study this.
> Purely since it is existing RCU kernel code that I'd like to understand. And
> me/Daniel are also looking into possibly using run-time / trace-based
> verification some of these behaviors.

The functionality of rcu_state.cbovld should make that more entertaining.

But I would guess that the initial model would ignore memory footprint
and just model RCU priority boosting as kicking in a fixed time after
the beginning of the grace period.

Or do you guys have something else in mind?

							Thanx, Paul

PS.  Steve, yes, I do well remember our earlier discussions about readers
     inheriting priority from the highest-priority synchronize_rcu().  ;-)

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Fri, Jan 17, 2020 at 08:34:58PM -0800, Paul E. McKenney wrote:
> On Fri, Jan 17, 2020 at 09:34:34PM -0500, Joel Fernandes wrote:
> > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > [...] 
> > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > failing in some way?
> > 
> > Yes there are tests, but I thought of just a simple experiment to study this.
> > Purely since it is existing RCU kernel code that I'd like to understand. And
> > me/Daniel are also looking into possibly using run-time / trace-based
> > verification some of these behaviors.
> 
> The functionality of rcu_state.cbovld should make that more entertaining.
> 
> But I would guess that the initial model would ignore memory footprint
> and just model RCU priority boosting as kicking in a fixed time after
> the beginning of the grace period.
> 
> Or do you guys have something else in mind?
> 
> 							Thanx, Paul
> 
> PS.  Steve, yes, I do well remember our earlier discussions about readers
>      inheriting priority from the highest-priority synchronize_rcu().  ;-)

To see the reason why RCU priority boosting does not work like that,
consider a (stupid but legal) situation with way more tasks like this
than the system can handle:

	for (;;) {
		cond_resched();
		p = kmalloc(sizeof(*p), GFP_ATOMIC);
		if (!p)
			continue;
		rcu_read_lock();
		kfree(&p->rh, my_func);
		rcu_read_unlock(;
	}

Nothing is ever waiting on the RCU grace period, so there is no natural
place for priority to be inherited from.

But the current RCU priority boosting works just fine in this situation,
at least assuming rcutree.kthread_prio is set suitably.  And if it is
not working fine in some other situation, it would be good for someone
to let me in on the secret.  ;-)

							Thanx, Paul

Re: RCU_BOOST not working for me

[Joel Fernandes]

On Fri, Jan 17, 2020 at 08:54:36PM -0800, Paul E. McKenney wrote:
> On Fri, Jan 17, 2020 at 08:34:58PM -0800, Paul E. McKenney wrote:
> > On Fri, Jan 17, 2020 at 09:34:34PM -0500, Joel Fernandes wrote:
> > > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > > [...] 
> > > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > > failing in some way?
> > > 
> > > Yes there are tests, but I thought of just a simple experiment to study this.
> > > Purely since it is existing RCU kernel code that I'd like to understand. And
> > > me/Daniel are also looking into possibly using run-time / trace-based
> > > verification some of these behaviors.
> > 
> > The functionality of rcu_state.cbovld should make that more entertaining.
> > 
> > But I would guess that the initial model would ignore memory footprint
> > and just model RCU priority boosting as kicking in a fixed time after
> > the beginning of the grace period.
> > 
> > Or do you guys have something else in mind?
> > 
> > 							Thanx, Paul
> > 
> > PS.  Steve, yes, I do well remember our earlier discussions about readers
> >      inheriting priority from the highest-priority synchronize_rcu().  ;-)
> 
> To see the reason why RCU priority boosting does not work like that,
> consider a (stupid but legal) situation with way more tasks like this
> than the system can handle:
> 
> 	for (;;) {
> 		cond_resched();
> 		p = kmalloc(sizeof(*p), GFP_ATOMIC);
> 		if (!p)
> 			continue;
> 		rcu_read_lock();
> 		kfree(&p->rh, my_func);
> 		rcu_read_unlock(;
> 	}
> 
> Nothing is ever waiting on the RCU grace period, so there is no natural
> place for priority to be inherited from.
> 
> But the current RCU priority boosting works just fine in this situation,
> at least assuming rcutree.kthread_prio is set suitably.  And if it is
> not working fine in some other situation, it would be good for someone
> to let me in on the secret.  ;-)

But in this example, you don't have anyone starting a grace period. So how
would the boosting be done? Somebody has to kick boosting into action?

thanks,

 - Joel

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Sat, Jan 18, 2020 at 03:21:31PM -0500, Joel Fernandes wrote:
> On Fri, Jan 17, 2020 at 08:54:36PM -0800, Paul E. McKenney wrote:
> > On Fri, Jan 17, 2020 at 08:34:58PM -0800, Paul E. McKenney wrote:
> > > On Fri, Jan 17, 2020 at 09:34:34PM -0500, Joel Fernandes wrote:
> > > > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > > > [...] 
> > > > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > > > failing in some way?
> > > > 
> > > > Yes there are tests, but I thought of just a simple experiment to study this.
> > > > Purely since it is existing RCU kernel code that I'd like to understand. And
> > > > me/Daniel are also looking into possibly using run-time / trace-based
> > > > verification some of these behaviors.
> > > 
> > > The functionality of rcu_state.cbovld should make that more entertaining.
> > > 
> > > But I would guess that the initial model would ignore memory footprint
> > > and just model RCU priority boosting as kicking in a fixed time after
> > > the beginning of the grace period.
> > > 
> > > Or do you guys have something else in mind?
> > > 
> > > 							Thanx, Paul
> > > 
> > > PS.  Steve, yes, I do well remember our earlier discussions about readers
> > >      inheriting priority from the highest-priority synchronize_rcu().  ;-)
> > 
> > To see the reason why RCU priority boosting does not work like that,
> > consider a (stupid but legal) situation with way more tasks like this
> > than the system can handle:
> > 
> > 	for (;;) {
> > 		cond_resched();
> > 		p = kmalloc(sizeof(*p), GFP_ATOMIC);
> > 		if (!p)
> > 			continue;
> > 		rcu_read_lock();
> > 		kfree(&p->rh, my_func);
> > 		rcu_read_unlock(;
> > 	}
> > 
> > Nothing is ever waiting on the RCU grace period, so there is no natural
> > place for priority to be inherited from.
> > 
> > But the current RCU priority boosting works just fine in this situation,
> > at least assuming rcutree.kthread_prio is set suitably.  And if it is
> > not working fine in some other situation, it would be good for someone
> > to let me in on the secret.  ;-)
> 
> But in this example, you don't have anyone starting a grace period. So how
> would the boosting be done? Somebody has to kick boosting into action?

Gah!

Right you are.  Please s/kfree/kfree_rcu/.

It was late and I was tired.  ;-)

							Thanx, Paul

Re: RCU_BOOST not working for me

[Joel Fernandes]

On Fri, Jan 17, 2020 at 08:34:58PM -0800, Paul E. McKenney wrote:
> On Fri, Jan 17, 2020 at 09:34:34PM -0500, Joel Fernandes wrote:
> > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > [...] 
> > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > failing in some way?
> > 
> > Yes there are tests, but I thought of just a simple experiment to study this.
> > Purely since it is existing RCU kernel code that I'd like to understand. And
> > me/Daniel are also looking into possibly using run-time / trace-based
> > verification some of these behaviors.
> 
> The functionality of rcu_state.cbovld should make that more entertaining.
> 
> But I would guess that the initial model would ignore memory footprint
> and just model RCU priority boosting as kicking in a fixed time after
> the beginning of the grace period.
> 
> Or do you guys have something else in mind?

Yes, that is the idea. And then turn the model into a unit test (for the
measurement). Though I am also personally trying to convince myself that a
unit test based on a model is better than the test in the kernel module I
just posted. We're just looking at applying Daniel's modeling work to
verification of behaviors like these.

A poor-man's alternative of a model-based test is just making sure that
synchronize_rcu() finishes in a bounded period of time (basically test by
observation than test by model) similar to what my kernel module did.  But I
guess a model based test would be more accurate and more strict about what is
considered a pass vs fail.

I was also studying SRCU and could not find tracepoints so I am thinking of
adding some to aid the study. I know for Tree-SRCU you are using timers and
workqueues but the concept hasn't largely changed since [1] was written
right?

[1] https://lwn.net/Articles/202847/

thanks!

 - Joel

> 							Thanx, Paul
> 
> PS.  Steve, yes, I do well remember our earlier discussions about readers
>      inheriting priority from the highest-priority synchronize_rcu().  ;-)

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Sat, Jan 18, 2020 at 03:19:37PM -0500, Joel Fernandes wrote:
> On Fri, Jan 17, 2020 at 08:34:58PM -0800, Paul E. McKenney wrote:
> > On Fri, Jan 17, 2020 at 09:34:34PM -0500, Joel Fernandes wrote:
> > > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > > [...] 
> > > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > > failing in some way?
> > > 
> > > Yes there are tests, but I thought of just a simple experiment to study this.
> > > Purely since it is existing RCU kernel code that I'd like to understand. And
> > > me/Daniel are also looking into possibly using run-time / trace-based
> > > verification some of these behaviors.
> > 
> > The functionality of rcu_state.cbovld should make that more entertaining.
> > 
> > But I would guess that the initial model would ignore memory footprint
> > and just model RCU priority boosting as kicking in a fixed time after
> > the beginning of the grace period.
> > 
> > Or do you guys have something else in mind?
> 
> Yes, that is the idea. And then turn the model into a unit test (for the
> measurement). Though I am also personally trying to convince myself that a
> unit test based on a model is better than the test in the kernel module I
> just posted. We're just looking at applying Daniel's modeling work to
> verification of behaviors like these.
> 
> A poor-man's alternative of a model-based test is just making sure that
> synchronize_rcu() finishes in a bounded period of time (basically test by
> observation than test by model) similar to what my kernel module did.  But I
> guess a model based test would be more accurate and more strict about what is
> considered a pass vs fail.

In one sense, fair enough.

But in a more practical sense, why would anyone put synchronize_rcu()
anywhere near their real-time fastpaths?  Even synchronize_rcu_expedited()
would be a rather brave choice for such a fastpath.

> I was also studying SRCU and could not find tracepoints so I am thinking of
> adding some to aid the study. I know for Tree-SRCU you are using timers and
> workqueues but the concept hasn't largely changed since [1] was written
> right?

At one point I had tracepoints for SRCU on my list, but the discussions
of tracepoints possibly being user API scared me off.

> [1] https://lwn.net/Articles/202847/

SRCU has been rewritten from scratch something like three times since
that article was published.  The current version is only a few years old.
And there is some motivation for more modifications due to the size of
the srcu_struct structure.  (Maybe dynamically allocating the srcu_node
array or some such, though this is not free of hazard and hassle, either.)
Thus far, all complaints about the large size have been handled by other
means, but there have been several such complaints.  In addition, the
use of workqueues is still a bit on the experimental side.  Looking good
thus far, but the code is yet young.

But yes, had the code remained unchanged for 14 years, there wouldn't
be much downside to adding tracepoints.  But the code is less than three
years old.

							Thanx, Paul

> thanks!
> 
>  - Joel
> 
> > 							Thanx, Paul
> > 
> > PS.  Steve, yes, I do well remember our earlier discussions about readers
> >      inheriting priority from the highest-priority synchronize_rcu().  ;-)

Re: RCU_BOOST not working for me

[Joel Fernandes]

On Sat, Jan 18, 2020 at 02:47:08PM -0800, Paul E. McKenney wrote:
> On Sat, Jan 18, 2020 at 03:19:37PM -0500, Joel Fernandes wrote:
> > On Fri, Jan 17, 2020 at 08:34:58PM -0800, Paul E. McKenney wrote:
> > > On Fri, Jan 17, 2020 at 09:34:34PM -0500, Joel Fernandes wrote:
> > > > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > > > [...] 
> > > > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > > > failing in some way?
> > > > 
> > > > Yes there are tests, but I thought of just a simple experiment to study this.
> > > > Purely since it is existing RCU kernel code that I'd like to understand. And
> > > > me/Daniel are also looking into possibly using run-time / trace-based
> > > > verification some of these behaviors.
> > > 
> > > The functionality of rcu_state.cbovld should make that more entertaining.
> > > 
> > > But I would guess that the initial model would ignore memory footprint
> > > and just model RCU priority boosting as kicking in a fixed time after
> > > the beginning of the grace period.
> > > 
> > > Or do you guys have something else in mind?
> > 
> > Yes, that is the idea. And then turn the model into a unit test (for the
> > measurement). Though I am also personally trying to convince myself that a
> > unit test based on a model is better than the test in the kernel module I
> > just posted. We're just looking at applying Daniel's modeling work to
> > verification of behaviors like these.
> > 
> > A poor-man's alternative of a model-based test is just making sure that
> > synchronize_rcu() finishes in a bounded period of time (basically test by
> > observation than test by model) similar to what my kernel module did.  But I
> > guess a model based test would be more accurate and more strict about what is
> > considered a pass vs fail.
> 
> In one sense, fair enough.
> 
> But in a more practical sense, why would anyone put synchronize_rcu()
> anywhere near their real-time fastpaths?  Even synchronize_rcu_expedited()
> would be a rather brave choice for such a fastpath.

Oh, I was just talking in the context of a unit test for boost, such as the
one I wrote. By measuring synchronize_rcu() time in the previous test I
wrote, we can get a sense of if the BOOST worked or not. Since the point of
BOOST is to shorten the otherwise lengthy grace period.

> > I was also studying SRCU and could not find tracepoints so I am thinking of
> > adding some to aid the study. I know for Tree-SRCU you are using timers and
> > workqueues but the concept hasn't largely changed since [1] was written
> > right?
> 
> At one point I had tracepoints for SRCU on my list, but the discussions
> of tracepoints possibly being user API scared me off.

I find it hard to imagine why any sane userspace tooling would want to depend
on RCU tracepoints. If they are just debug scripts like the one we've been
thinking of writing, then that's fine.

The latest on "tracepoints as user API" that I learnt from last conferences
is, if a tracepoint is so popular that userspace tools are using it and known
to use it, then that because ABI/API. If they are not used or unpopular, then
they are not so much as API. I believe the existing RCU tracepoints already
there, haven't shown to be an issue so may be it is Ok for RCU?

> SRCU has been rewritten from scratch something like three times since
> that article was published.  The current version is only a few years old.
> And there is some motivation for more modifications due to the size of
> the srcu_struct structure.  (Maybe dynamically allocating the srcu_node
> array or some such, though this is not free of hazard and hassle, either.)
> Thus far, all complaints about the large size have been handled by other
> means, but there have been several such complaints.  In addition, the
> use of workqueues is still a bit on the experimental side.  Looking good
> thus far, but the code is yet young.
> 
> But yes, had the code remained unchanged for 14 years, there wouldn't
> be much downside to adding tracepoints.  But the code is less than three
> years old.

Interesting! Thanks for the history.

 - Joel

Re: RCU_BOOST not working for me

[Paul E. McKenney]

On Sat, Jan 18, 2020 at 08:58:12PM -0500, Joel Fernandes wrote:
> On Sat, Jan 18, 2020 at 02:47:08PM -0800, Paul E. McKenney wrote:
> > On Sat, Jan 18, 2020 at 03:19:37PM -0500, Joel Fernandes wrote:
> > > On Fri, Jan 17, 2020 at 08:34:58PM -0800, Paul E. McKenney wrote:
> > > > On Fri, Jan 17, 2020 at 09:34:34PM -0500, Joel Fernandes wrote:
> > > > > On Fri, Jan 17, 2020 at 03:17:56PM -0800, Paul E. McKenney wrote:
> > > > > [...] 
> > > > > > But rcutorture already has tests for RCU priority boosting.  Or are those
> > > > > > failing in some way?
> > > > > 
> > > > > Yes there are tests, but I thought of just a simple experiment to study this.
> > > > > Purely since it is existing RCU kernel code that I'd like to understand. And
> > > > > me/Daniel are also looking into possibly using run-time / trace-based
> > > > > verification some of these behaviors.
> > > > 
> > > > The functionality of rcu_state.cbovld should make that more entertaining.
> > > > 
> > > > But I would guess that the initial model would ignore memory footprint
> > > > and just model RCU priority boosting as kicking in a fixed time after
> > > > the beginning of the grace period.
> > > > 
> > > > Or do you guys have something else in mind?
> > > 
> > > Yes, that is the idea. And then turn the model into a unit test (for the
> > > measurement). Though I am also personally trying to convince myself that a
> > > unit test based on a model is better than the test in the kernel module I
> > > just posted. We're just looking at applying Daniel's modeling work to
> > > verification of behaviors like these.
> > > 
> > > A poor-man's alternative of a model-based test is just making sure that
> > > synchronize_rcu() finishes in a bounded period of time (basically test by
> > > observation than test by model) similar to what my kernel module did.  But I
> > > guess a model based test would be more accurate and more strict about what is
> > > considered a pass vs fail.
> > 
> > In one sense, fair enough.
> > 
> > But in a more practical sense, why would anyone put synchronize_rcu()
> > anywhere near their real-time fastpaths?  Even synchronize_rcu_expedited()
> > would be a rather brave choice for such a fastpath.
> 
> Oh, I was just talking in the context of a unit test for boost, such as the
> one I wrote. By measuring synchronize_rcu() time in the previous test I
> wrote, we can get a sense of if the BOOST worked or not. Since the point of
> BOOST is to shorten the otherwise lengthy grace period.

OK, agreed, for testing this makes much more sense.  ;-)

> > > I was also studying SRCU and could not find tracepoints so I am thinking of
> > > adding some to aid the study. I know for Tree-SRCU you are using timers and
> > > workqueues but the concept hasn't largely changed since [1] was written
> > > right?
> > 
> > At one point I had tracepoints for SRCU on my list, but the discussions
> > of tracepoints possibly being user API scared me off.
> 
> I find it hard to imagine why any sane userspace tooling would want to depend
> on RCU tracepoints. If they are just debug scripts like the one we've been
> thinking of writing, then that's fine.
> 
> The latest on "tracepoints as user API" that I learnt from last conferences
> is, if a tracepoint is so popular that userspace tools are using it and known
> to use it, then that because ABI/API. If they are not used or unpopular, then
> they are not so much as API. I believe the existing RCU tracepoints already
> there, haven't shown to be an issue so may be it is Ok for RCU?

Still, caution seems warranted, at least until the code accumulates
a bit more time.

> > SRCU has been rewritten from scratch something like three times since
> > that article was published.  The current version is only a few years old.
> > And there is some motivation for more modifications due to the size of
> > the srcu_struct structure.  (Maybe dynamically allocating the srcu_node
> > array or some such, though this is not free of hazard and hassle, either.)
> > Thus far, all complaints about the large size have been handled by other
> > means, but there have been several such complaints.  In addition, the
> > use of workqueues is still a bit on the experimental side.  Looking good
> > thus far, but the code is yet young.
> > 
> > But yes, had the code remained unchanged for 14 years, there wouldn't
> > be much downside to adding tracepoints.  But the code is less than three
> > years old.
> 
> Interesting! Thanks for the history.

History I can provide in abundance.  ;-)

							Thanx, Paul