Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1270393 > unrolled thread

Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain Thomas Gleixner <tglx@linutronix.de> - 2015-11-16 18:20 +0100
    Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain Qais Yousef <qais.yousef@imgtec.com> - 2015-11-17 11:10 +0100
      Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain Thomas Gleixner <tglx@linutronix.de> - 2015-11-17 11:20 +0100
        Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain Qais Yousef <qais.yousef@imgtec.com> - 2015-11-17 11:40 +0100
    Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain Qais Yousef <qais.yousef@imgtec.com> - 2015-11-20 11:50 +0100
      Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domaind Thomas Gleixner <tglx@linutronix.de> - 2015-11-20 21:50 +0100
        Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domaind Qais Yousef <qais.yousef@imgtec.com> - 2015-11-23 18:00 +0100

#1270393 — Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain

On Mon, 9 Nov 2015, Qais Yousef wrote:
> On 11/07/2015 02:51 PM, Thomas Gleixner wrote:
> Generally it's hard to know whether a real device is connected to a hwirq or
> not. I am saving a patch where we get a set of free hwirqs from DT as only the
> SoC designer knows what hwirq are actually free and safe to use for IPI. I'll
> send this patch with the DT IPI changes or the rproc driver that I will be
> send once these changes are merged.
> 
> The current code assumes that the last 2 * NR_CPUs hwirqs are always free to
> use for Linux SMP.

So what you're saying is that you cannot rely on the last X hwirqs
being available for IPIs. That's insane and to my knowledge there is
no hardware out there which does not reserve a consecutive IPI space.

But nevertheless, lets look at the various (possible) requirements we
have:

1) IPI as per_cpu interrupts

   Single hwirq represented by a single irq descriptor

2) IPI with consecutive mapping space

   No extra mapping from virq base to target cpu required as its just
   linear. Everything can be handled via the base virq.

3) IPI with random mapping space

   Seperate mapping virq base to target cpu is required. The obvious
   place to store it are the irq descriptors. That needs a bit
   different machinery for ipi_send_mask(), but it's not rocket
   science.

> > That makes a lot of things simpler. You don't have to keep a mapping
> > of the hwirq to the target cpu. You just can use the base hwirq and
> > calculate the destination hwirq from there when sending an IPI
> > (general Linux ones). The coprocessor one will just be a natural
> > fallout.
> 
> Are you suggesting here to remove the whole new mapping API from the
> generic code or just that it's not necessary to use it in my case?

Err. I'm saying that you did not make use of hierarchical domains. You
just glued the IPI stuff sideways on the GIC.

We certainly want the generic code for managing the allocation etc.
 
> I'm confused here as well. Is this a complementary API or are you suggesting
> replacing the one this patch introduces?

Those are replacements. We just need to handle the random mapping case
if we really need it.
 
Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1271040

On 11/16/2015 05:17 PM, Thomas Gleixner wrote:
> On Mon, 9 Nov 2015, Qais Yousef wrote:
>> On 11/07/2015 02:51 PM, Thomas Gleixner wrote:
>> Generally it's hard to know whether a real device is connected to a hwirq or
>> not. I am saving a patch where we get a set of free hwirqs from DT as only the
>> SoC designer knows what hwirq are actually free and safe to use for IPI. I'll
>> send this patch with the DT IPI changes or the rproc driver that I will be
>> send once these changes are merged.
>>
>> The current code assumes that the last 2 * NR_CPUs hwirqs are always free to
>> use for Linux SMP.
> So what you're saying is that you cannot rely on the last X hwirqs
> being available for IPIs. That's insane and to my knowledge there is
> no hardware out there which does not reserve a consecutive IPI space.

If I read the code you were suggesting correctly, you were trying to fit 
the IPIs in any available non allocated area in the GIC space. What I am 
trying to say is that we can only work on a limited subset of this space 
that we are told explicitly it's safe to use for IPIs. Most likely it's 
consecutive, but I don't feel brave enough to make this assumption 
personally - maybe I'm over paranoid.. I'm more keen on anything that 
would simplify this patch series now though.

I'll do my best with the next series but maybe we'd need to iterate this 
more than once till I get it right.

Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1271044

On Tue, 17 Nov 2015, Qais Yousef wrote:
> On 11/16/2015 05:17 PM, Thomas Gleixner wrote:
> > On Mon, 9 Nov 2015, Qais Yousef wrote:
> > > On 11/07/2015 02:51 PM, Thomas Gleixner wrote:
> > > Generally it's hard to know whether a real device is connected to a hwirq
> > > or
> > > not. I am saving a patch where we get a set of free hwirqs from DT as only
> > > the
> > > SoC designer knows what hwirq are actually free and safe to use for IPI.
> > > I'll
> > > send this patch with the DT IPI changes or the rproc driver that I will be
> > > send once these changes are merged.
> > > 
> > > The current code assumes that the last 2 * NR_CPUs hwirqs are always free
> > > to
> > > use for Linux SMP.
> > So what you're saying is that you cannot rely on the last X hwirqs
> > being available for IPIs. That's insane and to my knowledge there is
> > no hardware out there which does not reserve a consecutive IPI space.
> 
> If I read the code you were suggesting correctly, you were trying to fit the
> IPIs in any available non allocated area in the GIC space. What I am trying to
> say is that we can only work on a limited subset of this space that we are
> told explicitly it's safe to use for IPIs. Most likely it's consecutive, but I
> don't feel brave enough to make this assumption personally - maybe I'm over
> paranoid.. I'm more keen on anything that would simplify this patch series now
> though.

Right, I was assuming a consecutive available space and your hardware
folks should really avoid to break that assumption.

Now you still need some DT support to describe the space which is
available for IPIs and that should be part of that series.

> I'll do my best with the next series but maybe we'd need to iterate
> this more than once till I get it right.

Thanks for being patient and persistant on that!

       tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1271057

On 11/17/2015 10:11 AM, Thomas Gleixner wrote:
> Right, I was assuming a consecutive available space and your hardware
> folks should really avoid to break that assumption.
>
> Now you still need some DT support to describe the space which is
> available for IPIs and that should be part of that series.


It's a simple change that shouldn't be a problem adding it to this 
series. I just wanted to avoid having to take more acks from more 
maintainers for this series to go in. But if it's needed, then it is 
what it is.

Maybe I'm better off sending this change separately actually as it's 
independent from other changes and could be merged in first.

> Thanks for being patient and persistant on that!

Thanks a lot for your help and patience too!

Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1273969

Hi Thomas,

On 11/16/2015 05:17 PM, Thomas Gleixner wrote:
> 1) IPI as per_cpu interrupts
>
>     Single hwirq represented by a single irq descriptor
>
> 2) IPI with consecutive mapping space
>
>     No extra mapping from virq base to target cpu required as its just
>     linear. Everything can be handled via the base virq.
>


I think I am seeing a major issue with this approach.

Take the case where we reserve an IPI with ipi_mask that has cpu 5 and 6 
set only. When allocating a per_cpu or consectuve mapping, we will 
require 2 consecutive virqs and hwirqs. But since the cpu location is 
not starting from 0, we can't use the cpu as an offset anymore.

So when a user wants to send an IPI to cpu 6 only, the code can't easily 
tell what's the correct offset from base virq or hwirq to use.

Same applies when doing the reverse mapping.

In other words, the ipi_mask won't always necessarily be linear to 
facilitate the 1:1 mapping that this approach assumes.

It is a solvable problem, but I think we're losing the elegance that 
promoted going into this direction and I think sticking to using struct 
ipi_mapping (with some enhancements to how it's exposed an integrated 
by/into generic code) is a better approach.

Thoughts?

I still don't have a working implementation otherwise I would have sent 
my patches, but I thought I'd raise this up before I spend more time on 
it unnecessarily.

Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274408 — Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domaind

Qais,

On Fri, 20 Nov 2015, Qais Yousef wrote:
> On 11/16/2015 05:17 PM, Thomas Gleixner wrote:
> > 1) IPI as per_cpu interrupts
> > 
> >     Single hwirq represented by a single irq descriptor
> > 
> > 2) IPI with consecutive mapping space
> > 
> >     No extra mapping from virq base to target cpu required as its just
> >     linear. Everything can be handled via the base virq.
> > 
> 
> 
> I think I am seeing a major issue with this approach.
> 
> Take the case where we reserve an IPI with ipi_mask that has cpu 5 and 6 set
> only. When allocating a per_cpu or consectuve mapping, we will require 2
> consecutive virqs and hwirqs. But since the cpu location is not starting from
> 0, we can't use the cpu as an offset anymore.
> 
> So when a user wants to send an IPI to cpu 6 only, the code can't easily tell
> what's the correct offset from base virq or hwirq to use.

Well, you can store the start offset easily and subtract it. It's 0
for most of the cases.

> Same applies when doing the reverse mapping.
> 
> In other words, the ipi_mask won't always necessarily be linear to facilitate
> the 1:1 mapping that this approach assumes.
> 
> It is a solvable problem, but I think we're losing the elegance that promoted
> going into this direction and I think sticking to using struct ipi_mapping
> (with some enhancements to how it's exposed an integrated by/into generic
> code) is a better approach.

The only reason to use the ipi_mapping thing is if we need non
consecutive masks, i.e. cpu 5 and 9.

I really don't want to have it mandatory as it does not make any sense
for systems where the IPI is a single per_cpu interrupt. For the
linear consecutive space it is just adding memory and cache footprint
for no benefit. Think about machines with 4k and more cpus ....

If you make ipi_mapping in a way that it can express the per_cpu,
linear and scattered mappings, then we should be fine. The extra
conditional you need in send_ipi() is not a problem.

Thanks,

	tglx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1275635 — Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domaind

On 11/20/2015 08:39 PM, Thomas Gleixner wrote:
>> Same applies when doing the reverse mapping.
>>
>> In other words, the ipi_mask won't always necessarily be linear to facilitate
>> the 1:1 mapping that this approach assumes.
>>
>> It is a solvable problem, but I think we're losing the elegance that promoted
>> going into this direction and I think sticking to using struct ipi_mapping
>> (with some enhancements to how it's exposed an integrated by/into generic
>> code) is a better approach.
> The only reason to use the ipi_mapping thing is if we need non
> consecutive masks, i.e. cpu 5 and 9.

That's the case I had in mind.

>
> I really don't want to have it mandatory as it does not make any sense
> for systems where the IPI is a single per_cpu interrupt. For the
> linear consecutive space it is just adding memory and cache footprint
> for no benefit. Think about machines with 4k and more cpus ....

OK. Although so far I think the ovehead is higher without the 
ipi_mapping because of all the extra checkings we have to do when 
sending an IPI. I'll leave this to code review when I have something 
ready though.

I'm debugging more problems and hopefully I'll send something this week.

Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web