Messages in this thread | | | Date | Fri, 24 Jul 2020 17:46:18 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH] sched,tracing: Convert to sched_set_fifo() |
| |
On Fri, 24 Jul 2020 23:39:11 +0200 peterz@infradead.org wrote:
> On Mon, Jul 20, 2020 at 11:49:18PM +0200, Peter Zijlstra wrote: > > Steve, would this work for you, or would you prefer renaming the > > parameters as well? > > Steve mentioned he's like to have the parameters changes after all. > How's this then? > > --- > Subject: sched,tracing: Convert to sched_set_fifo() > From: Peter Zijlstra <peterz@infradead.org> > Date: Mon, 20 Jul 2020 23:49:18 +0200 > > One module user of sched_setscheduler() was overlooked and is > obviously causing build failures. > > Convert ring_buffer_benchmark to use sched_set_fifo_low() when fifo==1 > and sched_set_fifo() when fifo==2. This is a bit of an abuse, but it > makes the thing 'work' again. > > Specifically, it enables all combinations that were previously > possible: > > producer higher than consumer > consumer higher than producer > > Fixes: 616d91b68cd5 ("sched: Remove sched_setscheduler*() EXPORTs") > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > kernel/trace/ring_buffer_benchmark.c | 48 ++++++++++++++++------------------- > 1 file changed, 23 insertions(+), 25 deletions(-) > > --- a/kernel/trace/ring_buffer_benchmark.c > +++ b/kernel/trace/ring_buffer_benchmark.c > @@ -45,8 +45,8 @@ MODULE_PARM_DESC(write_iteration, "# of > static int producer_nice = MAX_NICE; > static int consumer_nice = MAX_NICE; > > -static int producer_fifo = -1; > -static int consumer_fifo = -1; > +static int producer_fifo = 0; > +static int consumer_fifo = 0;
The initialization of zero for static variables isn't needed.
> > module_param(producer_nice, int, 0644); > MODULE_PARM_DESC(producer_nice, "nice prio for producer"); > @@ -55,10 +55,10 @@ module_param(consumer_nice, int, 0644); > MODULE_PARM_DESC(consumer_nice, "nice prio for consumer"); > > module_param(producer_fifo, int, 0644); > -MODULE_PARM_DESC(producer_fifo, "fifo prio for producer"); > +MODULE_PARM_DESC(producer_fifo, "use fifo for producer: 0 - disabled, 1 - low prio, 2 - fifo"); > > module_param(consumer_fifo, int, 0644); > -MODULE_PARM_DESC(consumer_fifo, "fifo prio for consumer"); > +MODULE_PARM_DESC(consumer_fifo, "use fifo for consumer: 0 - disabled, 1 - low prio, 2 - fifo"); > > static int read_events; > > @@ -303,22 +303,22 @@ static void ring_buffer_producer(void) > trace_printk("ERROR!\n"); > > if (!disable_reader) { > - if (consumer_fifo < 0) > - trace_printk("Running Consumer at nice: %d\n", > - consumer_nice); > - else > + if (consumer_fifo) > trace_printk("Running Consumer at SCHED_FIFO %d\n", > consumer_fifo);
Can we change the above to:
trace_printk("Running Consumer at SCHED_FIFO %s\n", consumer_fifo == 1 ? "low" : "high");
> + else > + trace_printk("Running Consumer at nice: %d\n", > + consumer_nice); > } > - if (producer_fifo < 0) > - trace_printk("Running Producer at nice: %d\n", > - producer_nice); > - else > + if (producer_fifo) > trace_printk("Running Producer at SCHED_FIFO %d\n", > producer_fifo);
Same here.
> + else > + trace_printk("Running Producer at nice: %d\n", > + producer_nice); > > /* Let the user know that the test is running at low priority */ > - if (producer_fifo < 0 && consumer_fifo < 0 && > + if (!producer_fifo && !consumer_fifo && > producer_nice == MAX_NICE && consumer_nice == MAX_NICE) > trace_printk("WARNING!!! This test is running at lowest priority.\n"); > > @@ -455,21 +455,19 @@ static int __init ring_buffer_benchmark_ > * Run them as low-prio background tasks by default: > */ > if (!disable_reader) { > - if (consumer_fifo >= 0) { > - struct sched_param param = { > - .sched_priority = consumer_fifo > - }; > - sched_setscheduler(consumer, SCHED_FIFO, ¶m); > - } else > + if (consumer_fifo == 2)
Let's make this be:
if (consumer_fifo > 1)
Then if someone sends in 3 it doesn't go low again.
> + sched_set_fifo(consumer); > + else if (consumer_fifo == 1) > + sched_set_fifo_low(consumer); > + else > set_user_nice(consumer, consumer_nice); > } > > - if (producer_fifo >= 0) { > - struct sched_param param = { > - .sched_priority = producer_fifo > - }; > - sched_setscheduler(producer, SCHED_FIFO, ¶m); > - } else > + if (producer_fifo == 2)
Same here.
-- Steve
> + sched_set_fifo(producer); > + else if (producer_fifo == 1) > + sched_set_fifo_low(producer); > + else > set_user_nice(producer, producer_nice); > > return 0;
| |