Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On 9/24/20 10:27 AM, pet...@infradead.org wrote: > So my current todo list is: > > - Change RT PULL > - Change DL PULL > - Add migrate_disable() tracer; exactly like preempt/irqoff, except >measuring task-runtime instead of cpu-time. > - Add a mode that measures actual interference. > - Add a traceevent to detect preemption in migrate_disable(). > > > And then I suppose I should twist Daniel's arm to update his model to > include these scenarios and numbers. Challenge accepted :-) -- Daniel ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, 24 Sep 2020 19:55:10 +0200 Thomas Gleixner wrote: > On Thu, Sep 24 2020 at 08:32, Steven Rostedt wrote: > > On Thu, 24 Sep 2020 08:57:52 +0200 > > Thomas Gleixner wrote: > > > >> > Now as for migration disabled nesting, at least now we would have > >> > groupings of this, and perhaps the theorists can handle that. I mean, > >> > how is this much different that having a bunch of tasks blocked on a > >> > mutex with the owner is pinned on a CPU? > >> > > >> > migrate_disable() is a BKL of pinning affinity. > >> > >> No. That's just wrong. preempt disable is a concurrency control, > > > > I think you totally misunderstood what I was saying. The above wasn't about > > comparing preempt_disable to migrate_disable. It was comparing > > migrate_disable to a chain of tasks blocked on mutexes where the top owner > > has preempt_disable set. You still have a bunch of tasks that can't move to > > other CPUs. > > What? The top owner does not prevent any task from moving. The tasks > cannot move because they are blocked on the mutex, which means they are > not runnable and non runnable tasks are not migrated at all. And neither are migrated disabled tasks preempted by a high priority task. > > I really don't understand what you are trying to say. Don't worry about it. I was just making a high level comparison of how migrate disabled tasks blocked on a higher priority task is similar to that of tasks blocked on a mutex held by a pinned task that is preempted by a high priority task. But we can forget this analogy as it's not appropriate for the current conversation. > > >> > If we only have local_lock() available (even on !RT), then it makes > >> > the blocking in groups. At least this way you could grep for all the > >> > different local_locks in the system and plug that into the algorithm > >> > for WCS, just like one would with a bunch of mutexes. > >> > >> You cannot do that on RT at all where migrate disable is substituting > >> preempt disable in spin and rw locks. The result would be the same as > >> with a !RT kernel just with horribly bad performance. > > > > Note, the spin and rwlocks already have a lock associated with them. Why > > would it be any different on RT? I wasn't suggesting adding another lock > > inside a spinlock. Why would I recommend THAT? I wasn't recommending > > blindly replacing migrate_disable() with local_lock(). I just meant expose > > local_lock() but not migrate_disable(). > > We already exposed local_lock() to non RT and it's for places which do > preempt_disable() or local_irq_disable() without having a lock > associated. But both primitives are scope less and therefore behave like > CPU local BKLs. What local_lock() provides in these cases is: > > - Making the protection scope clear by associating a named local > lock which is coverred by lockdep. > > - It still maps to preempt_disable() or local_irq_disable() in !RT > kernels > > - The scope and the named lock allows RT kernels to substitute with > real (recursion aware) locking primitives which keep preemption and > interupts enabled, but provide the fine grained protection for the > scoped critical section. I'm very much aware of the above. > > So how would you substitute migrate_disable() with a local_lock()? You > can't. Again migrate_disable() is NOT a concurrency control and > therefore it cannot be substituted by any concurrency control primitive. When I was first writing my email, I was writing about a way to replace migrate_disable with a construct similar to local locks without actually mentioning local locks, but then rewrote it to state local locks, trying to simplify what I was writing. I shouldn't have done that, because it portrayed that I wanted to use local_lock() unmodified. I was actually thinking of a new construct that was similar but not exactly the same as local lock. But this will just make things more complex and we can forget about it. I'll wait to see what Peter produces. -- Steve ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, Sep 24 2020 at 08:32, Steven Rostedt wrote: > On Thu, 24 Sep 2020 08:57:52 +0200 > Thomas Gleixner wrote: > >> > Now as for migration disabled nesting, at least now we would have >> > groupings of this, and perhaps the theorists can handle that. I mean, >> > how is this much different that having a bunch of tasks blocked on a >> > mutex with the owner is pinned on a CPU? >> > >> > migrate_disable() is a BKL of pinning affinity. >> >> No. That's just wrong. preempt disable is a concurrency control, > > I think you totally misunderstood what I was saying. The above wasn't about > comparing preempt_disable to migrate_disable. It was comparing > migrate_disable to a chain of tasks blocked on mutexes where the top owner > has preempt_disable set. You still have a bunch of tasks that can't move to > other CPUs. What? The top owner does not prevent any task from moving. The tasks cannot move because they are blocked on the mutex, which means they are not runnable and non runnable tasks are not migrated at all. I really don't understand what you are trying to say. >> > If we only have local_lock() available (even on !RT), then it makes >> > the blocking in groups. At least this way you could grep for all the >> > different local_locks in the system and plug that into the algorithm >> > for WCS, just like one would with a bunch of mutexes. >> >> You cannot do that on RT at all where migrate disable is substituting >> preempt disable in spin and rw locks. The result would be the same as >> with a !RT kernel just with horribly bad performance. > > Note, the spin and rwlocks already have a lock associated with them. Why > would it be any different on RT? I wasn't suggesting adding another lock > inside a spinlock. Why would I recommend THAT? I wasn't recommending > blindly replacing migrate_disable() with local_lock(). I just meant expose > local_lock() but not migrate_disable(). We already exposed local_lock() to non RT and it's for places which do preempt_disable() or local_irq_disable() without having a lock associated. But both primitives are scope less and therefore behave like CPU local BKLs. What local_lock() provides in these cases is: - Making the protection scope clear by associating a named local lock which is coverred by lockdep. - It still maps to preempt_disable() or local_irq_disable() in !RT kernels - The scope and the named lock allows RT kernels to substitute with real (recursion aware) locking primitives which keep preemption and interupts enabled, but provide the fine grained protection for the scoped critical section. So how would you substitute migrate_disable() with a local_lock()? You can't. Again migrate_disable() is NOT a concurrency control and therefore it cannot be substituted by any concurrency control primitive. >> That means the stacking problem has to be solved anyway. >> >> So why on earth do you want to create yet another special duct tape case >> for kamp_local() which proliferates inconsistency instead of aiming for >> consistency accross all preemption models? > > The idea was to help with the scheduling issue. I don't see how mixing concepts and trying to duct tape a problem which is clearly in the realm of the scheduler, i.e. load balancing and placing algorithms, is helpful. Thanks, tglx ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, Sep 24, 2020 at 09:51:38AM -0400, Steven Rostedt wrote: > > It turns out, that getting selected for pull-balance is exactly that > > condition, and clearly a migrate_disable() task cannot be pulled, but we > > can use that signal to try and pull away the running task that's in the > > way. > > Unless of course that running task is in a migrate disable section > itself ;-) See my ramblings here: https://lkml.kernel.org/r/20200924082717.ga1362...@hirez.programming.kicks-ass.net My plan was to have the migrate_enable() of the running task trigger the migration in that case. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, 24 Sep 2020 14:42:41 +0200 Peter Zijlstra wrote: > On Thu, Sep 24, 2020 at 08:32:41AM -0400, Steven Rostedt wrote: > > Anyway, instead of blocking. What about having a counter of number of > > migrate disabled tasks per cpu, and when taking a migrate_disable(), and > > there's > > already another task with migrate_disabled() set, and the current task has > > an affinity greater than 1, it tries to migrate to another CPU? > > That doesn't solve the problem. On wakeup we should already prefer an > idle CPU over one running a (RT) task, but you can always wake more > tasks than there's CPUs around and you'll _have_ to stack at some point. Yes, understood. > > The trick is how to unstack them correctly. We need to detect when a > migrate_disable() task _should_ start running again, and migrate away > whoever is in the way at that point. > > It turns out, that getting selected for pull-balance is exactly that > condition, and clearly a migrate_disable() task cannot be pulled, but we > can use that signal to try and pull away the running task that's in the > way. Unless of course that running task is in a migrate disable section itself ;-) But I guess we will always have that SHC, and there will always be a scenario that you can't balance properly. But hopefully in practice we wont see that. How to handle kmap_local(), will migrate_disable() be used only for 32bit or, for consistency, will it also apply to 64bit? -- Steve ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, Sep 24, 2020 at 08:32:41AM -0400, Steven Rostedt wrote: > Anyway, instead of blocking. What about having a counter of number of > migrate disabled tasks per cpu, and when taking a migrate_disable(), and > there's > already another task with migrate_disabled() set, and the current task has > an affinity greater than 1, it tries to migrate to another CPU? That doesn't solve the problem. On wakeup we should already prefer an idle CPU over one running a (RT) task, but you can always wake more tasks than there's CPUs around and you'll _have_ to stack at some point. The trick is how to unstack them correctly. We need to detect when a migrate_disable() task _should_ start running again, and migrate away whoever is in the way at that point. It turns out, that getting selected for pull-balance is exactly that condition, and clearly a migrate_disable() task cannot be pulled, but we can use that signal to try and pull away the running task that's in the way. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Thu, 24 Sep 2020 08:57:52 +0200 Thomas Gleixner wrote: > > Now as for migration disabled nesting, at least now we would have > > groupings of this, and perhaps the theorists can handle that. I mean, > > how is this much different that having a bunch of tasks blocked on a > > mutex with the owner is pinned on a CPU? > > > > migrate_disable() is a BKL of pinning affinity. > > No. That's just wrong. preempt disable is a concurrency control, I think you totally misunderstood what I was saying. The above wasn't about comparing preempt_disable to migrate_disable. It was comparing migrate_disable to a chain of tasks blocked on mutexes where the top owner has preempt_disable set. You still have a bunch of tasks that can't move to other CPUs. > > If we only have local_lock() available (even on !RT), then it makes > > the blocking in groups. At least this way you could grep for all the > > different local_locks in the system and plug that into the algorithm > > for WCS, just like one would with a bunch of mutexes. > > You cannot do that on RT at all where migrate disable is substituting > preempt disable in spin and rw locks. The result would be the same as > with a !RT kernel just with horribly bad performance. Note, the spin and rwlocks already have a lock associated with them. Why would it be any different on RT? I wasn't suggesting adding another lock inside a spinlock. Why would I recommend THAT? I wasn't recommending blindly replacing migrate_disable() with local_lock(). I just meant expose local_lock() but not migrate_disable(). > > That means the stacking problem has to be solved anyway. > > So why on earth do you want to create yet another special duct tape case > for kamp_local() which proliferates inconsistency instead of aiming for > consistency accross all preemption models? The idea was to help with the scheduling issue. Anyway, instead of blocking. What about having a counter of number of migrate disabled tasks per cpu, and when taking a migrate_disable(), and there's already another task with migrate_disabled() set, and the current task has an affinity greater than 1, it tries to migrate to another CPU? This way migrate_disable() is less likely to have a bunch of tasks blocked on one CPU serialized by each task exiting the migrate_disable() section. Yes, there's more overhead, but it only happens if multiple tasks are in a migrate disable section on the same CPU. -- Steve ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Wed, Sep 23, 2020 at 11:52:51AM -0400, Steven Rostedt wrote: > On Wed, 23 Sep 2020 10:40:32 +0200 > pet...@infradead.org wrote: > > > However, with migrate_disable() we can have each task preempted in a > > migrate_disable() region, worse we can stack them all on the _same_ CPU > > (super ridiculous odds, sure). And then we end up only able to run one > > task, with the rest of the CPUs picking their nose. > > What if we just made migrate_disable() a local_lock() available for !RT? Can't, neiter migrate_disable() nor migrate_enable() are allowed to block -- which is what makes their implementation so 'interesting'. > This should lower the SHC in theory, if you can't have stacked migrate > disables on the same CPU. See this email in that other thread (you're on Cc too IIRC): https://lkml.kernel.org/r/20200923170809.gy1362...@hirez.programming.kicks-ass.net I think that is we 'frob' the balance PULL, we'll end up with something similar. Whichever way around we turn this thing, the migrate_disable() runtime (we'll have to add a tracer for that), will be an interference term on the lower priority task, exactly like preempt_disable() would be. We'll just not exclude a higher priority task from running. AFAICT; the best case is a single migrate_disable() nesting, where a higher priority task preempts in a migrate_disable() section -- this is per design. When this preempted task becomes elegible to run under the ideal model (IOW it becomes one of the M highest priority tasks), it might still have to wait for the preemptee's migrate_disable() section to complete. Thereby suffering interference in the exact duration of migrate_disable() section. Per this argument, the change from preempt_disable() to migrate_disable() gets us: - higher priority tasks gain reduced wake-up latency - lower priority tasks are unchanged and are subject to the exact same interference term as if the higher priority task were using preempt_disable(). Since we've already established this term is unbounded, any task but the highest priority task is basically buggered. TL;DR, if we get balancing fixed and achieve (near) the optimal case above, migrate_disable() is an over-all win. But it's provably non-deterministic as long as the migrate_disable() sections are non-deterministic. The reason this all mostly works in practise is (I think) because: - People care most about the higher prio RT tasks and craft them to mostly avoid the migrate_disable() infected code. - The preemption scenario is most pronounced at higher utilization scenarios, and I suspect this is fairly rare to begin with. - And while many of these migrate_disable() regions are unbound in theory, in practise they're often fairly reasonable. So my current todo list is: - Change RT PULL - Change DL PULL - Add migrate_disable() tracer; exactly like preempt/irqoff, except measuring task-runtime instead of cpu-time. - Add a mode that measures actual interference. - Add a traceevent to detect preemption in migrate_disable(). And then I suppose I should twist Daniel's arm to update his model to include these scenarios and numbers. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v6 4/6] dt-bindings: dw-apb-ictl: support hierarchy irq domain
Add support to use dw-apb-ictl as primary interrupt controller.
Signed-off-by: Zhen Lei
Reviewed-by: Rob Herring
---
.../bindings/interrupt-controller/snps,dw-apb-ictl.txt | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
index 086ff08322db94f..2db59df9408f4c6 100644
---
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
+++
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
@@ -2,7 +2,8 @@ Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
Synopsys DesignWare provides interrupt controller IP for APB known as
dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
-APB bus, e.g. Marvell Armada 1500.
+APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
+controller in some SoCs, e.g. Hisilicon SD5203.
Required properties:
- compatible: shall be "snps,dw-apb-ictl"
@@ -10,6 +11,8 @@ Required properties:
region starting with ENABLE_LOW register
- interrupt-controller: identifies the node as an interrupt controller
- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be
1
+
+Additional required property when it's used as secondary interrupt controller:
- interrupts: interrupt reference to primary interrupt controller
The interrupt sources map to the corresponding bits in the interrupt
@@ -21,6 +24,7 @@ registers, i.e.
- (optional) fast interrupts start at 64.
Example:
+ /* dw_apb_ictl is used as secondary interrupt controller */
aic: interrupt-controller@3000 {
compatible = "snps,dw-apb-ictl";
reg = <0x3000 0xc00>;
@@ -29,3 +33,11 @@ Example:
interrupt-parent = <>;
interrupts = ;
};
+
+ /* dw_apb_ictl is used as primary interrupt controller */
+ vic: interrupt-controller@1013 {
+ compatible = "snps,dw-apb-ictl";
+ reg = <0x1013 0x1000>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
--
1.8.3
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v6 0/6] irqchip: dw-apb-ictl: support hierarchy irq domain
v5 --> v6: 1. add Reviewed-by: Rob Herring for Patch 4. 2. Some modifications are made to Patch 5: 1) add " |" for each "description:" property if its content exceeds one line, to tell the yaml keep the "newline" character. 2) add "..." to mark the end of the yaml file. 3) Change the name list of maintainers to the author of "snps,dw-apb-ictl.txt" maintainers: - - Marc Zyngier + - Sebastian Hesselbarth 4) add "maxItems: 1" for property "reg". 5) for property "interrupts": interrupts: -minItems: 1 -maxItems: 65 +maxItems: 1 6) move below descriptions under the top level property "description:" description: | Synopsys DesignWare provides interrupt controller IP for APB known as dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt controller in some SoCs, e.g. Hisilicon SD5203. + The interrupt sources map to the corresponding bits in the interrupt + registers, i.e. + - 0 maps to bit 0 of low interrupts, + - 1 maps to bit 1 of low interrupts, + - 32 maps to bit 0 of high interrupts, + - 33 maps to bit 1 of high interrupts, + - (optional) fast interrupts start at 64. + For more details of 2-6), please refer https://lkml.org/lkml/2020/9/24/13 v4 --> v5: 1. Add WARN_ON(1) in set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER 2. Convert "snps,dw-apb-ictl.txt" to "snps,dw-apb-ictl.yaml" 3. Fix the errors detected by "snps,dw-apb-ictl.yaml" on arch/arc v3 --> v4: 1. remove "gc->chip_types[0].chip.irq_eoi = irq_gc_noop;", the "chip.irq_eoi" hook is not needed by handle_level_irq(). Thanks for Marc Zyngier's review. 2. Add a new patch: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER to avoid compilation error on arch/arc system. v2 --> v3: 1. change (1 << hwirq) to BIT(hwirq). 2. change __exception_irq_entry to __irq_entry, so we can "#include " instead of "#include ". Ohterwise, an compilation error will be reported on arch/csky. drivers/irqchip/irq-dw-apb-ictl.c:20:10: fatal error: asm/exception.h: No such file or directory 3. use "if (!parent || (np == parent))" to determine whether it is primary interrupt controller. 4. make the primary interrupt controller case also use function handle_level_irq(), I used handle_fasteoi_irq() as flow_handler before. 5. Other minor changes are not detailed. v1 --> v2: According to Marc Zyngier's suggestion, discard adding an independent SD5203-VIC driver, but make the dw-apb-ictl irqchip driver to support hierarchy irq domain. It was originally available only for secondary interrupt controller, now it can also be used as primary interrupt controller. The related dt-bindings is updated appropriately. Add "Suggested-by: Marc Zyngier ". Add "Tested-by: Haoyu Lv ". v1: The interrupt controller of SD5203 SoC is VIC(vector interrupt controller), it's based on Synopsys DesignWare APB interrupt controller (dw_apb_ictl) IP, but it can not directly use dw_apb_ictl driver. The main reason is that VIC is used as primary interrupt controller and dw_apb_ictl driver worked for secondary interrupt controller. So add a new driver: "hisilicon,sd5203-vic". Zhen Lei (6): genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER irqchip: dw-apb-ictl: prepare for support hierarchy irq domain irqchip: dw-apb-ictl: support hierarchy irq domain dt-bindings: dw-apb-ictl: support hierarchy irq domain dt-bindings: dw-apb-ictl: convert to json-schema ARC: [dts] fix the errors detected by dtbs_check .../interrupt-controller/snps,dw-apb-ictl.txt | 31 .../interrupt-controller/snps,dw-apb-ictl.yaml | 74 +++ arch/arc/boot/dts/axc001.dtsi | 2 +- arch/arc/boot/dts/axc003.dtsi | 2 +- arch/arc/boot/dts/axc003_idu.dtsi | 2 +- arch/arc/boot/dts/vdk_axc003.dtsi | 2 +- arch/arc/boot/dts/vdk_axc003_idu.dtsi | 2 +- drivers/irqchip/Kconfig| 2 +- drivers/irqchip/irq-dw-apb-ictl.c | 83 ++ include/linux/irq.h| 6 ++ 10 files changed, 157 insertions(+), 49 deletions(-) delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt create mode 100644 Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml -- 1.8.3 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v6 6/6] ARC: [dts] fix the errors detected by dtbs_check
xxx/arc/boot/dts/axs101.dt.yaml: dw-apb-ictl@e0012000: $nodename:0: \
'dw-apb-ictl@e0012000' does not match '^interrupt-controller(@[0-9a-f,]+)*$'
>From schema: xxx/interrupt-controller/snps,dw-apb-ictl.yaml
The node name of the interrupt controller must start with
"interrupt-controller" instead of "dw-apb-ictl".
Signed-off-by: Zhen Lei
---
arch/arc/boot/dts/axc001.dtsi | 2 +-
arch/arc/boot/dts/axc003.dtsi | 2 +-
arch/arc/boot/dts/axc003_idu.dtsi | 2 +-
arch/arc/boot/dts/vdk_axc003.dtsi | 2 +-
arch/arc/boot/dts/vdk_axc003_idu.dtsi | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arc/boot/dts/axc001.dtsi b/arch/arc/boot/dts/axc001.dtsi
index 79ec27c043c1da7..2a151607b08057c 100644
--- a/arch/arc/boot/dts/axc001.dtsi
+++ b/arch/arc/boot/dts/axc001.dtsi
@@ -91,7 +91,7 @@
* avoid duplicating the MB dtsi file given that IRQ from
* this intc to cpu intc are different for axs101 and axs103
*/
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/axc003.dtsi b/arch/arc/boot/dts/axc003.dtsi
index ac8e1b463a70992..cd1edcf4f95efe6 100644
--- a/arch/arc/boot/dts/axc003.dtsi
+++ b/arch/arc/boot/dts/axc003.dtsi
@@ -129,7 +129,7 @@
* avoid duplicating the MB dtsi file given that IRQ from
* this intc to cpu intc are different for axs101 and axs103
*/
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/axc003_idu.dtsi
b/arch/arc/boot/dts/axc003_idu.dtsi
index 9da21e7fd246f9f..70779386ca7963a 100644
--- a/arch/arc/boot/dts/axc003_idu.dtsi
+++ b/arch/arc/boot/dts/axc003_idu.dtsi
@@ -135,7 +135,7 @@
* avoid duplicating the MB dtsi file given that IRQ from
* this intc to cpu intc are different for axs101 and axs103
*/
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0x0 0xe0012000 0x0 0x200 >;
diff --git a/arch/arc/boot/dts/vdk_axc003.dtsi
b/arch/arc/boot/dts/vdk_axc003.dtsi
index f8be7ba8dad499c..c21d0eb07bf6737 100644
--- a/arch/arc/boot/dts/vdk_axc003.dtsi
+++ b/arch/arc/boot/dts/vdk_axc003.dtsi
@@ -46,7 +46,7 @@
};
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0xe0012000 0x200 >;
diff --git a/arch/arc/boot/dts/vdk_axc003_idu.dtsi
b/arch/arc/boot/dts/vdk_axc003_idu.dtsi
index 0afa3e53a4e3932..4d348853ac7c5dc 100644
--- a/arch/arc/boot/dts/vdk_axc003_idu.dtsi
+++ b/arch/arc/boot/dts/vdk_axc003_idu.dtsi
@@ -54,7 +54,7 @@
};
- mb_intc: dw-apb-ictl@e0012000 {
+ mb_intc: interrupt-controller@e0012000 {
#interrupt-cells = <1>;
compatible = "snps,dw-apb-ictl";
reg = < 0xe0012000 0x200 >;
--
1.8.3
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v6 2/6] irqchip: dw-apb-ictl: prepare for support hierarchy irq domain
Rename some functions and variables in advance, to make the next patch
looks more clear. The details are as follows:
1. rename dw_apb_ictl_handler() to dw_apb_ictl_handle_irq_cascaded().
2. change (1 << hwirq) to BIT(hwirq).
In function dw_apb_ictl_init():
1. rename local variable irq to parent_irq.
2. add "const struct irq_domain_ops *domain_ops = _generic_chip_ops",
then replace _generic_chip_ops in other places with domain_ops.
No functional change.
Signed-off-by: Zhen Lei
Tested-by: Haoyu Lv
---
drivers/irqchip/irq-dw-apb-ictl.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c
b/drivers/irqchip/irq-dw-apb-ictl.c
index e4550e9c810ba94..5458004242e9d20 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -26,7 +26,7 @@
#define APB_INT_FINALSTATUS_H 0x34
#define APB_INT_BASE_OFFSET0x04
-static void dw_apb_ictl_handler(struct irq_desc *desc)
+static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc *desc)
{
struct irq_domain *d = irq_desc_get_handler_data(desc);
struct irq_chip *chip = irq_desc_get_chip(desc);
@@ -43,7 +43,7 @@ static void dw_apb_ictl_handler(struct irq_desc *desc)
u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
generic_handle_irq(virq);
- stat &= ~(1 << hwirq);
+ stat &= ~BIT(hwirq);
}
}
@@ -73,12 +73,13 @@ static int __init dw_apb_ictl_init(struct device_node *np,
struct irq_domain *domain;
struct irq_chip_generic *gc;
void __iomem *iobase;
- int ret, nrirqs, irq, i;
+ int ret, nrirqs, parent_irq, i;
u32 reg;
+ const struct irq_domain_ops *domain_ops = _generic_chip_ops;
/* Map the parent interrupt for the chained handler */
- irq = irq_of_parse_and_map(np, 0);
- if (irq <= 0) {
+ parent_irq = irq_of_parse_and_map(np, 0);
+ if (parent_irq <= 0) {
pr_err("%pOF: unable to parse irq\n", np);
return -EINVAL;
}
@@ -120,8 +121,7 @@ static int __init dw_apb_ictl_init(struct device_node *np,
else
nrirqs = fls(readl_relaxed(iobase + APB_INT_ENABLE_L));
- domain = irq_domain_add_linear(np, nrirqs,
- _generic_chip_ops, NULL);
+ domain = irq_domain_add_linear(np, nrirqs, domain_ops, NULL);
if (!domain) {
pr_err("%pOF: unable to add irq domain\n", np);
ret = -ENOMEM;
@@ -146,7 +146,8 @@ static int __init dw_apb_ictl_init(struct device_node *np,
gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
}
- irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
+ irq_set_chained_handler_and_data(parent_irq,
+ dw_apb_ictl_handle_irq_cascaded, domain);
return 0;
--
1.8.3
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v6 5/6] dt-bindings: dw-apb-ictl: convert to json-schema
Convert the Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
binding to DT schema format using json-schema.
Signed-off-by: Zhen Lei
---
.../interrupt-controller/snps,dw-apb-ictl.txt | 43 -
.../interrupt-controller/snps,dw-apb-ictl.yaml | 74 ++
2 files changed, 74 insertions(+), 43 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
deleted file mode 100644
index 2db59df9408f4c6..000
---
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
-
-Synopsys DesignWare provides interrupt controller IP for APB known as
-dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
-APB bus, e.g. Marvell Armada 1500. It can also be used as primary interrupt
-controller in some SoCs, e.g. Hisilicon SD5203.
-
-Required properties:
-- compatible: shall be "snps,dw-apb-ictl"
-- reg: physical base address of the controller and length of memory mapped
- region starting with ENABLE_LOW register
-- interrupt-controller: identifies the node as an interrupt controller
-- #interrupt-cells: number of cells to encode an interrupt-specifier, shall be
1
-
-Additional required property when it's used as secondary interrupt controller:
-- interrupts: interrupt reference to primary interrupt controller
-
-The interrupt sources map to the corresponding bits in the interrupt
-registers, i.e.
-- 0 maps to bit 0 of low interrupts,
-- 1 maps to bit 1 of low interrupts,
-- 32 maps to bit 0 of high interrupts,
-- 33 maps to bit 1 of high interrupts,
-- (optional) fast interrupts start at 64.
-
-Example:
- /* dw_apb_ictl is used as secondary interrupt controller */
- aic: interrupt-controller@3000 {
- compatible = "snps,dw-apb-ictl";
- reg = <0x3000 0xc00>;
- interrupt-controller;
- #interrupt-cells = <1>;
- interrupt-parent = <>;
- interrupts = ;
- };
-
- /* dw_apb_ictl is used as primary interrupt controller */
- vic: interrupt-controller@1013 {
- compatible = "snps,dw-apb-ictl";
- reg = <0x1013 0x1000>;
- interrupt-controller;
- #interrupt-cells = <1>;
- };
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
new file mode 100644
index 000..1b05d36b5f7b943
--- /dev/null
+++
b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/snps,dw-apb-ictl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
+
+maintainers:
+ - Sebastian Hesselbarth
+
+description: |
+ Synopsys DesignWare provides interrupt controller IP for APB known as
+ dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs
+ with APB bus, e.g. Marvell Armada 1500. It can also be used as primary
+ interrupt controller in some SoCs, e.g. Hisilicon SD5203.
+
+ The interrupt sources map to the corresponding bits in the interrupt
+ registers, i.e.
+ - 0 maps to bit 0 of low interrupts,
+ - 1 maps to bit 1 of low interrupts,
+ - 32 maps to bit 0 of high interrupts,
+ - 33 maps to bit 1 of high interrupts,
+ - (optional) fast interrupts start at 64.
+
+allOf:
+ - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+ compatible:
+const: snps,dw-apb-ictl
+
+ interrupt-controller: true
+
+ reg:
+description: |
+ Physical base address of the controller and length of memory mapped
+ region starting with ENABLE_LOW register.
+maxItems: 1
+
+ interrupts:
+description: Interrupt reference to primary interrupt controller.
+maxItems: 1
+
+ "#interrupt-cells":
+description: Number of cells to encode an interrupt-specifier.
+const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupt-controller
+ - '#interrupt-cells'
+
+examples:
+ - |
+/* dw_apb_ictl is used as secondary interrupt controller */
+aic: interrupt-controller@3000 {
+compatible = "snps,dw-apb-ictl";
+reg = <0x3000 0xc00>;
+interrupt-controller;
+#interrupt-cells = <1>;
+interrupt-parent = <>;
+interrupts = <0 3 4>;
+};
+
+/* dw_apb_ictl is used as primary interrupt controller */
+vic:
[PATCH v6 3/6] irqchip: dw-apb-ictl: support hierarchy irq domain
Add support to use dw-apb-ictl as primary interrupt controller.
Suggested-by: Marc Zyngier
Signed-off-by: Zhen Lei
Tested-by: Haoyu Lv
---
drivers/irqchip/Kconfig | 2 +-
drivers/irqchip/irq-dw-apb-ictl.c | 74 ++-
2 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index bfc9719dbcdc31c..7c2d1c8fa551a66 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -148,7 +148,7 @@ config DAVINCI_CP_INTC
config DW_APB_ICTL
bool
select GENERIC_IRQ_CHIP
- select IRQ_DOMAIN
+ select IRQ_DOMAIN_HIERARCHY
config FARADAY_FTINTC010
bool
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c
b/drivers/irqchip/irq-dw-apb-ictl.c
index 5458004242e9d20..418183b9983dfad 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#define APB_INT_ENABLE_L 0x00
#define APB_INT_ENABLE_H 0x04
@@ -26,6 +27,27 @@
#define APB_INT_FINALSTATUS_H 0x34
#define APB_INT_BASE_OFFSET0x04
+/* irq domain of the primary interrupt controller. */
+static struct irq_domain *dw_apb_ictl_irq_domain;
+
+static void __irq_entry dw_apb_ictl_handle_irq(struct pt_regs *regs)
+{
+ struct irq_domain *d = dw_apb_ictl_irq_domain;
+ int n;
+
+ for (n = 0; n < d->revmap_size; n += 32) {
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, n);
+ u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
+
+ while (stat) {
+ u32 hwirq = ffs(stat) - 1;
+
+ handle_domain_irq(d, hwirq, regs);
+ stat &= ~BIT(hwirq);
+ }
+ }
+}
+
static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc *desc)
{
struct irq_domain *d = irq_desc_get_handler_data(desc);
@@ -50,6 +72,30 @@ static void dw_apb_ictl_handle_irq_cascaded(struct irq_desc
*desc)
chained_irq_exit(chip, desc);
}
+static int dw_apb_ictl_irq_domain_alloc(struct irq_domain *domain, unsigned
int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ int i, ret;
+ irq_hw_number_t hwirq;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = arg;
+
+ ret = irq_domain_translate_onecell(domain, fwspec, , );
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++)
+ irq_map_generic_chip(domain, virq + i, hwirq + i);
+
+ return 0;
+}
+
+static const struct irq_domain_ops dw_apb_ictl_irq_domain_ops = {
+ .translate = irq_domain_translate_onecell,
+ .alloc = dw_apb_ictl_irq_domain_alloc,
+ .free = irq_domain_free_irqs_top,
+};
+
#ifdef CONFIG_PM
static void dw_apb_ictl_resume(struct irq_data *d)
{
@@ -75,13 +121,20 @@ static int __init dw_apb_ictl_init(struct device_node *np,
void __iomem *iobase;
int ret, nrirqs, parent_irq, i;
u32 reg;
- const struct irq_domain_ops *domain_ops = _generic_chip_ops;
-
- /* Map the parent interrupt for the chained handler */
- parent_irq = irq_of_parse_and_map(np, 0);
- if (parent_irq <= 0) {
- pr_err("%pOF: unable to parse irq\n", np);
- return -EINVAL;
+ const struct irq_domain_ops *domain_ops;
+
+ if (!parent || (np == parent)) {
+ /* It's used as the primary interrupt controller */
+ parent_irq = 0;
+ domain_ops = _apb_ictl_irq_domain_ops;
+ } else {
+ /* Map the parent interrupt for the chained handler */
+ parent_irq = irq_of_parse_and_map(np, 0);
+ if (parent_irq <= 0) {
+ pr_err("%pOF: unable to parse irq\n", np);
+ return -EINVAL;
+ }
+ domain_ops = _generic_chip_ops;
}
ret = of_address_to_resource(np, 0, );
@@ -146,8 +199,13 @@ static int __init dw_apb_ictl_init(struct device_node *np,
gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
}
- irq_set_chained_handler_and_data(parent_irq,
+ if (parent_irq) {
+ irq_set_chained_handler_and_data(parent_irq,
dw_apb_ictl_handle_irq_cascaded, domain);
+ } else {
+ dw_apb_ictl_irq_domain = domain;
+ set_handle_irq(dw_apb_ictl_handle_irq);
+ }
return 0;
--
1.8.3
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v6 1/6] genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER
To avoid compilation error if an irqchip driver references the function
set_handle_irq() but may not select GENERIC_IRQ_MULTI_HANDLER on some
systems.
For example, the Synopsys DesignWare APB interrupt controller
(dw_apb_ictl) is used as the secondary interrupt controller on arc, csky,
arm64, and most arm32 SoCs, and it's also used as the primary interrupt
controller on Hisilicon SD5203 (an arm32 SoC). The latter need to use
set_handle_irq() to register the top-level IRQ handler, but this multi
irq handler registration mechanism is not implemented on arc system.
The input parameter "handle_irq" maybe defined as static and only
set_handle_irq() references it. This will trigger "defined but not used"
warning. So add "(void)handle_irq" to suppress it.
Signed-off-by: Zhen Lei
---
include/linux/irq.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 1b7f4dfee35b397..b167baef88c0b43 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1252,6 +1252,12 @@ void irq_matrix_free(struct irq_matrix *m, unsigned int
cpu,
* top-level IRQ handler.
*/
extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+#else
+#define set_handle_irq(handle_irq) \
+ do {\
+ (void)handle_irq; \
+ WARN_ON(1); \
+ } while (0)
#endif
#endif /* _LINUX_IRQ_H */
--
1.8.3
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Wed, Sep 23 2020 at 17:12, Steven Rostedt wrote: > On Wed, 23 Sep 2020 22:55:54 +0200 > Then scratch the idea of having anonymous local_lock() and just bring > local_lock in directly? Then have a kmap local lock, which would only > block those that need to do a kmap. That's still going to end up in lock ordering nightmares and you lose the ability to use kmap_local from arbitrary contexts which was again one of the goals of this exercise. Aside of that you're imposing reentrancy protections on something which does not need it in the first place. > Now as for migration disabled nesting, at least now we would have > groupings of this, and perhaps the theorists can handle that. I mean, > how is this much different that having a bunch of tasks blocked on a > mutex with the owner is pinned on a CPU? > > migrate_disable() is a BKL of pinning affinity. No. That's just wrong. preempt disable is a concurrency control, i.e. protecting against reentrancy on a given CPU. But it's a cpu global protection which means that it's not protecting a specific code path. Contrary to preempt disable, migrate disable is not protecting against reentrancy on a given CPU. It's a temporary restriction to the scheduler on placement. The fact that disabling preemption implicitely disables migration does not make them semantically equivalent. > If we only have local_lock() available (even on !RT), then it makes > the blocking in groups. At least this way you could grep for all the > different local_locks in the system and plug that into the algorithm > for WCS, just like one would with a bunch of mutexes. You cannot do that on RT at all where migrate disable is substituting preempt disable in spin and rw locks. The result would be the same as with a !RT kernel just with horribly bad performance. That means the stacking problem has to be solved anyway. So why on earth do you want to create yet another special duct tape case for kamp_local() which proliferates inconsistency instead of aiming for consistency accross all preemption models? Thanks, tglx ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
