* Re: arch/tile
@ 2010-05-26 14:05 Thomas Gleixner
2010-05-28 17:37 ` arch/tile Chris Metcalf
0 siblings, 1 reply; 2+ messages in thread
From: Thomas Gleixner @ 2010-05-26 14:05 UTC (permalink / raw)
To: Chris Metcalf; +Cc: Linux Kernel Mailing List, Linus Torvalds
Chris,
On Tue, 25 May 2010, Chris Metcalf wrote:
> Thomas, thanks for your feedback. If I don't comment on something you
> said it's because you were obviously right and I applied a suitable fix. :-)
>
> On 5/25/2010 4:12 PM, Thomas Gleixner wrote:
> > +/**
> > + * tile_request_irq() - Allocate an interrupt handling instance.
> > [...]
> >
> > Why are you implementing your private interrupt handling
> > infrastructure ? What's wrong with the generic interrupt handling
> > code ? Why is each device driver forced to call tile_request_irq()
> > which makes it incompatible to the rest of the kernel and therefor
> > unshareable ?
> >
>
> Our interrupt management code has evolved as we have developed this
> code, so I won't present arguments as to why it's perfect the way it is,
> but just why it IS the way it is. :-)
>
> The tile irq.c does not replace the generic Linux IRQ management code,
> but instead provides a very limited set of virtual interrupts that are
> only used by our para-virtualized device drivers, and delivered to Linux
> via a single hypervisor downcall that atomically sets "virtual
> interrupt" bits in a bitmask. The PCI root complex driver reserves four
> of these bits (i.e. irqs) to map real PCI interrupts; they are then fed
> forward into the regular Linux IRQ system to manage all "standard"
> devices. The other tile-specific para-virtualized drivers that use this
> interface are the PCI endpoint code, xgbe network driver, ATA-over-GPIO
> driver, and the IPI layer. None of these para-virtualized drivers are
> actually shareable with other Linux architectures in any case.
Ok, still ...
> We have an outstanding enhancement request in our bug tracking system to
> switch to using the Linux generic IRQs directly, and plan to implement
> it prior to our next major release. But we haven't done it yet, and
> this code, though somewhat crufty, is reasonably stable. I'm also not
> the primary maintainer of this particular piece of code, so I'd rather
> wait until that person frees up and have him do it, instead of trying to
> hack it myself.
It should be rather straight forward and can be efficiently done
with the handle_simple_irq() handler AFAICS. So I'd prefer to see
that fixed befor the code gets merged.
> > You check desc->handler, but you happily call the handler while
> > dev_id might be still NULL. See above.
> >
>
> Assuming we spinlock the irq request/free routines, I think this is
> safe, since the unlock memory fence will guarantee visibility of the
> fields prior to any attempt to use them. We always allocate the
> interrupt, then tell the hypervisor to start delivering them; on device
> unload we tell the hypervisor to stop delivering interrupts, then free
> it. The "tell the hypervisor" steps use on_each_cpu() and wait, so are
> fully synchronous.
Going to generic irq will solve all of that :)
> > +int show_interrupts(struct seq_file *p, void *v)
> > +[...]
> >
> > So that prints which interrupts ? Now you refer to the generic code,
> > while above you require that tile specific one. -ENOSENSE.
> >
>
> Yes, this is confusing. This routine is required by procfs, and it
> shows just the PCI interrupts, not the tile irqs. I'll add a comment,
> and try to segregate the file into "generic irqs" and "tile irqs" more
> obviously, for now. The routine itself will be more sensible once we
> integrate our tile_irqs into the generic system.
Ditto.
Thanks,
tglx
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: arch/tile
2010-05-26 14:05 arch/tile Thomas Gleixner
@ 2010-05-28 17:37 ` Chris Metcalf
0 siblings, 0 replies; 2+ messages in thread
From: Chris Metcalf @ 2010-05-28 17:37 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Linux Kernel Mailing List, Linus Torvalds
On 5/26/2010 10:05 AM, Thomas Gleixner wrote:
>> We have an outstanding enhancement request in our bug tracking system to
>> switch to using the Linux generic IRQs directly, and plan to implement
>> it prior to our next major release. But we haven't done it yet, and
>> this code, though somewhat crufty, is reasonably stable. I'm also not
>> the primary maintainer of this particular piece of code, so I'd rather
>> wait until that person frees up and have him do it, instead of trying to
>> hack it myself.
>>
> It should be rather straight forward and can be efficiently done
> with the handle_simple_irq() handler AFAICS. So I'd prefer to see
> that fixed befor the code gets merged.
>
OK, we have made changes to integrate to the Linux PERCPU IRQ mechanism,
which most closely matches what our interrupt layer did. You should see
it a bit later when I post the revised diffs.
Thanks!
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-05-28 17:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-26 14:05 arch/tile Thomas Gleixner
2010-05-28 17:37 ` arch/tile Chris Metcalf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.