On Wed, Dec 03, 2014 at 08:39:47PM +0100, Luis R. Rodriguez wrote: > On Wed, Dec 03, 2014 at 05:37:51AM +0100, Juergen Gross wrote: > > On 12/03/2014 03:28 AM, Luis R. Rodriguez wrote: > >> On Tue, Dec 02, 2014 at 11:11:18AM +0000, David Vrabel wrote: > >>> On 01/12/14 22:36, Luis R. Rodriguez wrote: > >>>> > >>>> Then I do agree its a fair analogy (and find this obviously odd that how > >>>> widespread cond_resched() is), we just don't have an equivalent for IRQ > >>>> context, why not avoid the special check then and use this all the time in the > >>>> middle of a hypercall on the return from an interrupt (e.g., the timer > >>>> interrupt)? > >>> > >>> http://lists.xen.org/archives/html/xen-devel/2014-02/msg01101.html > >> > >> OK thanks! That explains why we need some asm code but in that submission you > >> still also had used is_preemptible_hypercall(regs) and in the new > >> implementation you use a CPU variable xen_in_preemptible_hcall prior to calling > >> preempt_schedule_irq(). I believe you added the CPU variable because > >> preempt_schedule_irq() will preempt first without any checks if it should, I'm > >> asking why not do something like cond_resched_irq() where we check with > >> should_resched() prior to preempting and that way we can avoid having to use > >> the CPU variable? > > > > Because that could preempt at any asynchronous interrupt making the > > no-preempt kernel fully preemptive. > > OK yeah I see. That still doesn't negate the value of using something > like cond_resched_irq() with a should_resched() on only critical hypercalls. > The current implementation (patch by David) forces preemption without > checking for should_resched() so it would preempt unnecessarily at least > once. > > > How would you know you are just > > doing a critical hypercall which should be preempted? > > You would not, you're right. I was just trying to see if we could generalize > an API for this to avoid having users having to create their own CPU variables > but this all seems very specialized as we want to use this on the timer > so if we do generalize a cond_resched_irq() perhaps the documentation can > warn about this type of case or abuse. David's patch had the check only it was x86 based, if we use cond_resched_irq() we can leave that aspect out to be done through asm inlines or it'll use the generic shoudl_resched(), that should save some code on the asm implementations. I have some patches now which generalizees this, I also have more information about this can happen exactly, and a way to triggger it on small systems with some hacks to emulate possibly backend behaviour on larger systems. In the worst case this can be a dangerious situation to be in. I'll send some new RFTs. Luis -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html