From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Miao Date: Fri, 08 Oct 2010 07:09:46 +0000 Subject: Re: [PATCH 00/08] ARM: Dynamic IRQ demux support Message-Id: List-Id: References: <20101006071731.28048.89938.sendpatchset@t400s> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-arm-kernel@lists.infradead.org On Thu, Oct 7, 2010 at 2:39 PM, Magnus Damm wrote: > Hi Eric, > > On Wed, Oct 6, 2010 at 10:06 PM, Eric Miao wrote: >> On Wed, Oct 6, 2010 at 3:17 PM, Magnus Damm wrote: >>> ARM: Dynamic IRQ demux support > >> Just FYI, I had a patch months ago for this: >> >> http://www.spinics.net/linux/lists/arm-kernel/msg92836.html >> >> Do you think that's a simpler way to go? > > Thanks for the pointer. I wasn't aware of your patch, but the fact > that both of us came up with similar solutions to the same problem > clearly shows that there is a need for this feature. > > Your patch is much less intrusive compared to what i came up with. I > like the simplicity. I guess it is natural that the simplicity comes > with a bit of overhead. I'm thinking of the extra branches and > whatever potential cache misses coming from the code and callback > being located in different cache lines compared to the rest of the > code in __irq_svc and __irq_usr. > > My patches keeps the handlers in the same cache lines as before, but > this comes with the cost of more serious rearrangement of the code. > Also, my alignment_trap patch may decrease performance, not sure how > to deal with that in a good way. Yeah, I was thinking of the performance degradation at that time as well. What worried me most is actually the long jump. However, we do have a long jump to asm_do_IRQ anyway. And the optimizations like get_irqnr_preamble and get_irqnr_and_base can be carried out in C code as well, or if that's tricky enough, inline assembly code can also help. > > I don't mind so much which patch that gets merged, but I'm a little > bit concerned about code duplication. My patch moves macros into a > header file that each demux instance can make use of to minimize the > amount of duplicated code. I'm not sure how you are planning on > implementing each demux instance. If possible I'd like to see the > irq_handler macro in a header file somewhere so we don't have to > duplicate that code. Machines will have to specify their IRQ demux handler, which could be same among a group of machines which share common demux code, and thus duplication can be avoided. > > On top of that I'd also like to break out the GIC demux code and put > it in a central location, but that's a separate issue. I think we can make a common function call for all the IRQ demux handler for GIC then. > > Any thoughts? Shall I try to move the irq_handler macro to some header file? > Your patch is excellent, I don't mind either which gets in. What I'm a bit concerned is the effort to keep up with the mostly optimized code. I was thinking of a bit (not that much significant I guess) performance loss when multiple IRQ demux handler is required for the sake of simplicity and less intrusiveness, while still able to use the original optimized path when that's not required (i.e. turning CONFIG_.... off). Thoughts? > Thanks, > > / magnus >