Messages in this thread | | | From | Doug Anderson <> | Date | Wed, 22 Apr 2020 09:41:43 -0700 | Subject | Re: [PATCH 11/23] sched,spi: Convert to sched_set_fifo*() |
| |
Hi,
On Wed, Apr 22, 2020 at 8:48 AM Guenter Roeck <groeck@google.com> wrote: > > On Wed, Apr 22, 2020 at 7:35 AM Doug Anderson <dianders@chromium.org> wrote: > > > > Hi, > > > > On Wed, Apr 22, 2020 at 6:56 AM Mark Brown <broonie@kernel.org> wrote: > > > > > > On Wed, Apr 22, 2020 at 01:27:30PM +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. > > > > > > > > No effective change. > > > > > > Copying Doug who did this change and Guenter who reviewed it. This > > > looks fine to me but I've no particular involvement with the code or > > > platforms that are affected here. > > > > Thanks! Probably the maintainers of cros_ec_spi.c (Benson and Enric) > > should be aware of it, too. CCing them. > > > > From my point of view, my response is pretty much identical to the one > > I wrote when the priority was reduced from "MAX_RT_PRIO - 1" to > > "MAX_RT_PRIO / 2" [1]. Basically, any priority that keeps us from > > being preempted by tasks that are only high priority for performance > > reasons (like dm crypt and loopback did when I last analyzed) is fine. > > Our priority needs to be high not for performance reasons but for > > correctness reasons (the other side will drop our data if we don't > > respond in time). > > > The crypto engine ends up running at the same priority level, so I am > a bit concerned that this patch series will re-introduce the problem > that Doug's initial patch tried to solve.
Do you have a pointer to the code you're looking at? Digging through my old investigation for dm-crypt showed the problem to be the code touched by commit a1b89132dc4f ("dm crypt: use WQ_HIGHPRI for the IO and crypt workqueues"). Interestingly enough, that's been reverted in commit f612b2132db5 ("Revert "dm crypt: use WQ_HIGHPRI for the IO and crypt workqueues"").
Even if something is using WQ_HIGHPRI, last I checked WQ_HIGHPRI was the highest non-relatime priority. Looking quickly I see "HIGHPRI_NICE_LEVEL" which is MIN_NICE. I don't think that implies realtime, but I assume sched_fifo() still does.
-Doug
| |