cc'ing Doug as he might be really interested on this patch
On 1/8/19 13:13, Peter Zijlstra wrote:
> The SPI thingies request FIFO-99 by default, reduce this to FIFO-50.
>
You say below that is not a suitable default but there is any other reason? Did
you observed problems with this?
> FIFO-99 is the very highest priority available to SCHED_FIFO and
> it not a suitable default; it would indicate the SPI work is the
> most important work on the machine.
>
> Cc: Benson Leung <[email protected]>
> Cc: Enric Balletbo i Serra <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> drivers/platform/chrome/cros_ec_spi.c | 2 +-
> drivers/spi/spi.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -706,7 +706,7 @@ static int cros_ec_spi_devm_high_pri_all
> struct cros_ec_spi *ec_spi)
> {
> struct sched_param sched_priority = {
> - .sched_priority = MAX_RT_PRIO - 1,
> + .sched_priority = MAX_RT_PRIO / 2,
> };
> int err;
>
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1434,7 +1434,7 @@ static void spi_pump_messages(struct kth
> */
> static void spi_set_thread_rt(struct spi_controller *ctlr)
> {
> - struct sched_param param = { .sched_priority = MAX_RT_PRIO - 1 };
> + struct sched_param param = { .sched_priority = MAX_RT_PRIO / 2 };
>
> dev_info(&ctlr->dev,
> "will run message pump with realtime priority\n");
>
>
On Thu, Aug 01, 2019 at 01:27:04PM +0200, Enric Balletbo i Serra wrote:
>
> cc'ing Doug as he might be really interested on this patch
>
> On 1/8/19 13:13, Peter Zijlstra wrote:
> > The SPI thingies request FIFO-99 by default, reduce this to FIFO-50.
> >
>
> You say below that is not a suitable default but there is any other reason? Did
> you observed problems with this?
I didn't observe any problems with it personally. But imagine your
device is used to control something critical, like something leathal,
say a bandsaw. And just as you stick your hand through the laser
guarding it, your SPI device chirps and preempts the task that was about
to pull the emergency break and save your hand.
FIFO-99 is the highest possible prio (for SCHED_FIFO) and by using it
you say your task is the utmost imporant task on the system.
I'm thinking that isn't true 99% of the time, except of course when that
bandsaw emergency break is attached through SPI, but in that case the
admin can very well chrt the prio of this thread.
On Thu, Aug 01, 2019 at 02:17:18PM +0200, Peter Zijlstra wrote:
> I'm thinking that isn't true 99% of the time, except of course when that
> bandsaw emergency break is attached through SPI, but in that case the
> admin can very well chrt the prio of this thread.
The SPI thread isn't usually RT, it's only made RT if something in the
system asks for it - the reason the ChromeOS people got CCed in is that
some of their embedded controllers are very fragile and need super tight
timing on some of the interactions over their control interface so
they're one of the users here. Of course everyone is then going to
claim that their usage is the most critical usage in the system, and
they may well even be right, but I do tend to agree that just any old RT
priority is probably a sensible default since for most cases there will
be few if any other RT tasks anyway.
Hi,
On Thu, Aug 1, 2019 at 5:35 AM Mark Brown <[email protected]> wrote:
>
> On Thu, Aug 01, 2019 at 02:17:18PM +0200, Peter Zijlstra wrote:
>
> > I'm thinking that isn't true 99% of the time, except of course when that
> > bandsaw emergency break is attached through SPI, but in that case the
> > admin can very well chrt the prio of this thread.
>
> The SPI thread isn't usually RT, it's only made RT if something in the
> system asks for it - the reason the ChromeOS people got CCed in is that
> some of their embedded controllers are very fragile and need super tight
> timing on some of the interactions over their control interface so
> they're one of the users here. Of course everyone is then going to
> claim that their usage is the most critical usage in the system, and
> they may well even be right, but I do tend to agree that just any old RT
> priority is probably a sensible default since for most cases there will
> be few if any other RT tasks anyway.
For the Chrome OS case I believe that "MAX_RT_PRIO / 2" should be just
fine. In fact in an earlier version of my work to make CrOS EC work
better at <https://crrev.com/c/1603464> I had said "We'll arbitrarily
pick a priority of "MAX_RT_PRIO / 4 - 1", AKA 24. This seems to work
fine in practice." I only switched to "MAX_RT_PRIO - 1" to match the
SPI code.
Mostly we just need to be a bit higher than things that request the
highest non-realtime priority, notably DM Crypt and loopback which
both schedule a bunch of work on the high priority system workqueue.
Those two things in particular seem to want high priority for
performance reasons but not for correctness reasons. As mentioned
earlier our EC will actually fail transfers if there is too much
delay.
Thus:
Reviewed-by: Douglas Anderson <[email protected]>