Re: [PATCH 05/23] sched,drbd: Convert to sched_set_fifo*()

From: Peter Zijlstra
Date: Thu Apr 23 2020 - 04:58:28 EST


On Wed, Apr 22, 2020 at 01:27:24PM +0200, Peter Zijlstra wrote:
> Because SCHED_FIFO is a broken scheduler model (see previous patches)
> take away the priority field, the kernel can't possibly make an
> informed decision.
>
> In this case, use fifo_low, because it only cares about being above
> SCHED_NORMAL. Effectively changes prio from 2 to 1.
>
> Cc: axboe@xxxxxxxxx
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Reviewed-by: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
> drivers/block/drbd/drbd_receiver.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> --- a/drivers/block/drbd/drbd_receiver.c
> +++ b/drivers/block/drbd/drbd_receiver.c
> @@ -6020,9 +6020,8 @@ int drbd_ack_receiver(struct drbd_thread
> unsigned int header_size = drbd_header_size(connection);
> int expect = header_size;
> bool ping_timeout_active = false;
> - struct sched_param param = { .sched_priority = 2 };
>
> - rv = sched_setscheduler(current, SCHED_RR, &param);
> + rv = sched_set_fifo_low(current);

As noted by Dietmar, I forgot to mention loosing RR in the changelog,
bad me.

In this case I'm not actually 100% sure, but there was no comment with
it that justified it being RR, and RR-SMP is not optimal (in fact it's
rather buggered).

In general RR is even more 'interesting' to get right thatn FIFO and
therefore I figured it probably didn't want to be RR, but given there
can be multiple of such threads, it might have been an attempt at
providing some sort of fairness between the multiple threads.

At the same time, if you're running the threads so hard that RR makes a
difference, it's unlikely there is any actual NORMAL time left and
things will be unhappy anyway.

Therefore, and me being lazy, make it FIFO.