Re: [PATCH] ipmi_si_intf: use usleep_range() instead of busy looping

From: Corey Minyard
Date: Tue Jul 09 2019 - 19:01:50 EST


On Tue, Jul 09, 2019 at 03:09:08PM -0700, Tejun Heo wrote:
> Hello, Corey.
>
> On Tue, Jul 09, 2019 at 04:46:02PM -0500, Corey Minyard wrote:
> > I'm also a little confused because the CPU in question shouldn't
> > be doing anything else if the schedule() immediately returns here,
> > so it's not wasting CPU that could be used on another process. Or
> > is it lock contention that is causing an issue on other CPUs?
>
> Yeah, pretty pronounced too and it also keeps the CPU busy which makes
> the load balancer deprioritize that CPU. Busy looping is never free.
>
> > IMHO, this whole thing is stupid; if you design hardware with
> > stupid interfaces (byte at a time, no interrupts) you should
> > expect to get bad performance. But I can't control what the
> > hardware vendors do. This whole thing is a carefully tuned
> > compromise.
>
> I'm really not sure "carefully tuned" is applicable on indefinite busy
> looping.

Well, yeah, but other things were tried and this was the only thing
we could find that worked. That was before the kind of SMP stuff
we have now, though.

>
> > So I can't really take this as-is.
>
> We can go for shorter timeouts for sure but I don't think this sort of
> busy looping is acceptable. Is your position that this must be a busy
> loop?

Well, no. I want something that provides as high a throughput as
possible and doesn't cause scheduling issues. But that may not be
possible. Screwing up the scheduler is a lot worse than slow IPMI
firmware updates.

How short can the timeouts be and avoid issues?

Thanks,

-corey

>
> Thanks.
>
> --
> tejun