2020-04-22 11:31:50

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH 23/23] sched: Remove sched_set_*() return value

Ingo suggested that since the new sched_set_*() functions are
implemented using the 'nocheck' variants, they really shouldn't ever
fail, so remove the return value.

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Suggested-by: Ingo Molnar <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
drivers/block/drbd/drbd_receiver.c | 4 +---
drivers/firmware/psci/psci_checker.c | 3 +--
drivers/gpu/drm/msm/msm_drv.c | 5 +----
drivers/platform/chrome/cros_ec_spi.c | 7 +++----
include/linux/sched.h | 6 +++---
kernel/rcu/rcutorture.c | 5 +----
kernel/sched/core.c | 12 ++++++------
7 files changed, 16 insertions(+), 26 deletions(-)

--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -6021,9 +6021,7 @@ int drbd_ack_receiver(struct drbd_thread
int expect = header_size;
bool ping_timeout_active = false;

- rv = sched_set_fifo_low(current);
- if (rv < 0)
- drbd_err(connection, "drbd_ack_receiver: ERROR set priority, ret=%d\n", rv);
+ sched_set_fifo_low(current);

while (get_t_state(thi) == RUNNING) {
drbd_thread_current_set_cpu(thi);
--- a/drivers/firmware/psci/psci_checker.c
+++ b/drivers/firmware/psci/psci_checker.c
@@ -281,8 +281,7 @@ static int suspend_test_thread(void *arg
wait_for_completion(&suspend_threads_started);

/* Set maximum priority to preempt all other threads on this CPU. */
- if (sched_set_fifo(current))
- pr_warn("Failed to set suspend thread scheduler on CPU %d\n", cpu);
+ sched_set_fifo(current);

dev = this_cpu_read(cpuidle_devices);
drv = cpuidle_get_cpu_driver(dev);
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -508,10 +508,7 @@ static int msm_drm_init(struct device *d
goto err_msm_uninit;
}

- ret = sched_set_fifo(priv->event_thread[i].thread);
- if (ret)
- dev_warn(dev, "event_thread set priority failed:%d\n",
- ret);
+ sched_set_fifo(priv->event_thread[i].thread);
}

ret = drm_vblank_init(ddev, priv->num_crtcs);
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -725,10 +725,9 @@ static int cros_ec_spi_devm_high_pri_all
if (err)
return err;

- err = sched_set_fifo(ec_spi->high_pri_worker->task);
- if (err)
- dev_err(dev, "Can't set cros_ec high pri priority: %d\n", err);
- return err;
+ sched_set_fifo(ec_spi->high_pri_worker->task);
+
+ return 0;
}

static int cros_ec_spi_probe(struct spi_device *spi)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1631,9 +1631,9 @@ extern int idle_cpu(int cpu);
extern int available_idle_cpu(int cpu);
extern int sched_setscheduler(struct task_struct *, int, const struct sched_param *);
extern int sched_setscheduler_nocheck(struct task_struct *, int, const struct sched_param *);
-extern int sched_set_fifo(struct task_struct *p);
-extern int sched_set_fifo_low(struct task_struct *p);
-extern int sched_set_normal(struct task_struct *p, int nice);
+extern void sched_set_fifo(struct task_struct *p);
+extern void sched_set_fifo_low(struct task_struct *p);
+extern void sched_set_normal(struct task_struct *p, int nice);
extern int sched_setattr(struct task_struct *, const struct sched_attr *);
extern int sched_setattr_nocheck(struct task_struct *, const struct sched_attr *);
extern struct task_struct *idle_task(int cpu);
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -806,10 +806,7 @@ static int rcu_torture_boost(void *arg)
VERBOSE_TOROUT_STRING("rcu_torture_boost started");

/* Set real-time priority. */
- if (sched_set_fifo_low(current) < 0) {
- VERBOSE_TOROUT_STRING("rcu_torture_boost RT prio failed!");
- n_rcu_torture_boost_rterror++;
- }
+ sched_set_fifo_low(current);

init_rcu_head_on_stack(&rbi.rcu);
/* Each pass through the following loop does one boost-test cycle. */
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5114,30 +5114,30 @@ int sched_setscheduler_nocheck(struct ta
* The administrator _MUST_ configure the system, the kernel simply doesn't
* know enough information to make a sensible choice.
*/
-int sched_set_fifo(struct task_struct *p)
+void sched_set_fifo(struct task_struct *p)
{
struct sched_param sp = { .sched_priority = MAX_RT_PRIO / 2 };
- return sched_setscheduler_nocheck(p, SCHED_FIFO, &sp);
+ WARN_ON_ONCE(sched_setscheduler_nocheck(p, SCHED_FIFO, &sp) != 0);
}
EXPORT_SYMBOL_GPL(sched_set_fifo);

/*
* For when you don't much care about FIFO, but want to be above SCHED_NORMAL.
*/
-int sched_set_fifo_low(struct task_struct *p)
+void sched_set_fifo_low(struct task_struct *p)
{
struct sched_param sp = { .sched_priority = 1 };
- return sched_setscheduler_nocheck(p, SCHED_FIFO, &sp);
+ WARN_ON_ONCE(sched_setscheduler_nocheck(p, SCHED_FIFO, &sp) != 0);
}
EXPORT_SYMBOL_GPL(sched_set_fifo_low);

-int sched_set_normal(struct task_struct *p, int nice)
+void sched_set_normal(struct task_struct *p, int nice)
{
struct sched_attr attr = {
.sched_policy = SCHED_NORMAL,
.sched_nice = nice,
};
- return sched_setattr_nocheck(p, &attr);
+ WARN_ON_ONCE(sched_setattr_nocheck(p, &attr) != 0);
}
EXPORT_SYMBOL_GPL(sched_set_normal);




2020-04-22 14:27:16

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 23/23] sched: Remove sched_set_*() return value


* Peter Zijlstra <[email protected]> wrote:

> Ingo suggested that since the new sched_set_*() functions are
> implemented using the 'nocheck' variants, they really shouldn't ever
> fail, so remove the return value.
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Suggested-by: Ingo Molnar <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>

Reviewed-by: Ingo Molnar <[email protected]>

Thanks,

Ingo

2020-04-22 16:21:13

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 23/23] sched: Remove sched_set_*() return value

On Wed, Apr 22, 2020 at 01:27:42PM +0200, Peter Zijlstra wrote:
> Ingo suggested that since the new sched_set_*() functions are
> implemented using the 'nocheck' variants, they really shouldn't ever
> fail, so remove the return value.
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Suggested-by: Ingo Molnar <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> drivers/block/drbd/drbd_receiver.c | 4 +---
> drivers/firmware/psci/psci_checker.c | 3 +--
> drivers/gpu/drm/msm/msm_drv.c | 5 +----
> drivers/platform/chrome/cros_ec_spi.c | 7 +++----
> include/linux/sched.h | 6 +++---
> kernel/rcu/rcutorture.c | 5 +----
> kernel/sched/core.c | 12 ++++++------
> 7 files changed, 16 insertions(+), 26 deletions(-)

[ . . . ]

> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -806,10 +806,7 @@ static int rcu_torture_boost(void *arg)
> VERBOSE_TOROUT_STRING("rcu_torture_boost started");
>
> /* Set real-time priority. */
> - if (sched_set_fifo_low(current) < 0) {
> - VERBOSE_TOROUT_STRING("rcu_torture_boost RT prio failed!");
> - n_rcu_torture_boost_rterror++;
> - }
> + sched_set_fifo_low(current);
>
> init_rcu_head_on_stack(&rbi.rcu);
> /* Each pass through the following loop does one boost-test cycle. */

This is the only update of n_rcu_torture_boost_rterror, so it can
be eliminated entirely, for example as shown below.

Other than that, looks good to me!

Thanx, Paul

------------------------------------------------------------------------

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index ee27b57..61b0c4f 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -171,7 +171,6 @@ static atomic_t n_rcu_torture_mberror;
static atomic_t n_rcu_torture_error;
static long n_rcu_torture_barrier_error;
static long n_rcu_torture_boost_ktrerror;
-static long n_rcu_torture_boost_rterror;
static long n_rcu_torture_boost_failure;
static long n_rcu_torture_boosts;
static atomic_long_t n_rcu_torture_timers;
@@ -893,10 +892,7 @@ static int rcu_torture_boost(void *arg)
VERBOSE_TOROUT_STRING("rcu_torture_boost started");

/* Set real-time priority. */
- if (sched_set_fifo_low(current) < 0) {
- VERBOSE_TOROUT_STRING("rcu_torture_boost RT prio failed!");
- n_rcu_torture_boost_rterror++;
- }
+ sched_set_fifo_low(current);

init_rcu_head_on_stack(&rbi.rcu);
/* Each pass through the following loop does one boost-test cycle. */
@@ -1527,11 +1523,10 @@ rcu_torture_stats_print(void)
atomic_read(&n_rcu_torture_alloc),
atomic_read(&n_rcu_torture_alloc_fail),
atomic_read(&n_rcu_torture_free));
- pr_cont("rtmbe: %d rtbe: %ld rtbke: %ld rtbre: %ld ",
+ pr_cont("rtmbe: %d rtbe: %ld rtbke: %ld ",
atomic_read(&n_rcu_torture_mberror),
n_rcu_torture_barrier_error,
- n_rcu_torture_boost_ktrerror,
- n_rcu_torture_boost_rterror);
+ n_rcu_torture_boost_ktrerror);
pr_cont("rtbf: %ld rtb: %ld nt: %ld ",
n_rcu_torture_boost_failure,
n_rcu_torture_boosts,
@@ -1545,14 +1540,12 @@ rcu_torture_stats_print(void)
pr_alert("%s%s ", torture_type, TORTURE_FLAG);
if (atomic_read(&n_rcu_torture_mberror) ||
n_rcu_torture_barrier_error || n_rcu_torture_boost_ktrerror ||
- n_rcu_torture_boost_rterror || n_rcu_torture_boost_failure ||
- i > 1) {
+ n_rcu_torture_boost_failure || i > 1) {
pr_cont("%s", "!!! ");
atomic_inc(&n_rcu_torture_error);
WARN_ON_ONCE(atomic_read(&n_rcu_torture_mberror));
WARN_ON_ONCE(n_rcu_torture_barrier_error); // rcu_barrier()
WARN_ON_ONCE(n_rcu_torture_boost_ktrerror); // no boost kthread
- WARN_ON_ONCE(n_rcu_torture_boost_rterror); // can't set RT prio
WARN_ON_ONCE(n_rcu_torture_boost_failure); // RCU boost failed
WARN_ON_ONCE(i > 1); // Too-short grace period
}
@@ -2569,7 +2562,6 @@ rcu_torture_init(void)
atomic_set(&n_rcu_torture_error, 0);
n_rcu_torture_barrier_error = 0;
n_rcu_torture_boost_ktrerror = 0;
- n_rcu_torture_boost_rterror = 0;
n_rcu_torture_boost_failure = 0;
n_rcu_torture_boosts = 0;
for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++)