Re: [Adeos-main] [pull request] x86: critical fix for native_safe_halt

[Philippe Gerum <rpm@xenomai.org>]

On Sun, 2009-12-13 at 19:19 +0100, Jan Kiszka wrote:
> Philippe Gerum wrote:
> > On Sun, 2009-12-13 at 18:48 +0100, Jan Kiszka wrote:
> >> Philippe Gerum wrote:
> >>> On Sat, 2009-12-12 at 22:37 +0100, Jan Kiszka wrote:
> >>>> Recent moving of ipipe_suspend_domain finally exposed a deeper flaw in
> >>>> cpu_idle on x86: We failed to check the pipeline log before issuing the
> >>>> real hlt. This caused IRQ latencies or even drops for Linux,
> >>>> specifically on SMP. Credits go to plain QEMU whose slow SMP mode caused
> >>>> ipipe_critical_enter to deadlock frequently enough.
> >>>>
> >>>> The first patch of this series fixes this (see below), the second one
> >>>> simply removes the two useless ipipe_suspend_domain calls.
> >>>>
> >>> What your patch does as well, is killing the ability to run low priority
> >>> domains below the root level.
> >> Yes, I'm killing the dream.
> >>
> >> I heavily doubt that the functions I removed in the second patch ever
> >> contributed something good to this. It's always the job of the lowest
> >> domain to issue hardware halt, not of some arbitrary mid-prio domain.

Actually, no it's not. You may use a low-priority domain to run idle
level jobs outside of the linux infrastructure for that purpose (e.g.
RCU). A high priority domain may want to post events for a low priority
domain to act upon when a mid priority domain is about to enter the CPU
idle state.

> >> Moreover, what would be the practical use for such model in the context
> >> of Linux?
> > 
> > That is _not_ the point. The point is, when submitting a patch, please
> > make sure to raise all the concerns it might introduce wrt to changing
> > the base features. I'm not opposed to make the feature set evolve, but I
> > don't want this to happen "by mistake".
> 
> Just pushed
> 
> "x86: Drop redundant ipipe_suspend_domain from cpu_idle
> 
> Allowing domains below root always required more than these calls (Linux
> would have to give up idle management). And syncing the root domain now
> takes place in __ipipe_halt_root. So remove these suspension calls."
> 
> as commit message for the second patch. Is that what you are looking for?
> 

Not exactly, because your comment states that what was removed was
intrinsically useless, it was not, and has been used, even if only in a
couple of occasions, mainly to enable a debugging hack. This is why I
choked on removing a feature silently. But at the same time, we are in a
simplification trend of the I-pipe toward X3, so I agree on the final
goal you are pursuing with that patch.

In short, let's move on, I'll merge that as well, now that everything is
on the table, and there is no objection anyway.

> Jan
> 
> _______________________________________________
> Adeos-main mailing list
> Adeos-main@domain.hid
> https://mail.gna.org/listinfo/adeos-main


-- 
Philippe.