Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Mon, Sep 21, 2020 at 09:27:57PM +0200, Thomas Gleixner wrote: > On Mon, Sep 21 2020 at 09:24, Linus Torvalds wrote: > > On Mon, Sep 21, 2020 at 12:39 AM Thomas Gleixner wrote: > >> > >> If a task is migrated to a different CPU then the mapping address will > >> change which will explode in colourful ways. > > > > Right you are. > > > > Maybe we really *could* call this new kmap functionality something > > like "kmap_percpu()" (or maybe "local" is good enough), and make it > > act like your RT code does for spinlocks - not disable preemption, but > > only disabling CPU migration. > > I"m all for it, but the scheduler people have opinions :) Right, so I'm concerned. migrate_disable() wrecks pretty much all Real-Time scheduler theory we have, and PREEMPRT_RT bringing it in is somewhat ironic. Yes, it allows breaking up non-preemptible regions of non-deterministic duration, and thereby both reduce and bound the scheduling latency, the cost for doing that is that the theory on CPU utilization/bandwidth go out the window. To easily see this consider an SMP system with a number of tasks equal to the number of CPUs. On a regular (preempt_disable) kernel we can always run each task, by virtue of always having an idle CPU to take the task. 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. The end result is that, like with unbounded latency, we will still miss our deadline, simply because we got starved for CPU. Now, while we could (with a _lot_ of work) rework the kernel to not rely on the implicit per-cpu ness of things like spinlock_t, the moment we bring in basic primitives that rely on migrate_disable() we're stuck with it. The problem is; afaict there's been no research into this problem. There might be scheduling (read: balancing) schemes that can mitigate/solve this problem, or it might prove to be a 'hard' problem, I just don't know. But once we start down this road, it's going to be hell to get rid of it. That's why I've been arguing (for many years) to strictly limit this to PREEMPT_RT and only as a gap-stop, not a fundamental primitive to build on. ___ 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 Mon, Sep 21, 2020 at 09:27:57PM +0200, Thomas Gleixner wrote: > Alternatively this could of course be solved with per CPU page tables > which will come around some day anyway I fear. Previously (with PTI) we looked at making the entire kernel map per-CPU, and that takes a 2K copy on switch_mm() (or more general, the user part of whatever the top level directory is for architectures that have a shared kernel/user page-table setup in the first place). The idea was having a fixed per-cpu kernel page-table, share a bunch of (kernel) page-tables between all CPUs and then copy in the user part on switch. I've forgotten what the plan was for ASID/PCID in that scheme. For x86_64 we've been fearing the performance of that 2k copy, but I don't think we've ever actually bit the bullet and implemented it to see how bad it really is. ___ 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 12:19, peterz wrote: > On Mon, Sep 21, 2020 at 09:27:57PM +0200, Thomas Gleixner wrote: >> Alternatively this could of course be solved with per CPU page tables >> which will come around some day anyway I fear. > > Previously (with PTI) we looked at making the entire kernel map per-CPU, > and that takes a 2K copy on switch_mm() (or more general, the user part > of whatever the top level directory is for architectures that have a > shared kernel/user page-table setup in the first place). > > The idea was having a fixed per-cpu kernel page-table, share a bunch of > (kernel) page-tables between all CPUs and then copy in the user part on > switch. > > I've forgotten what the plan was for ASID/PCID in that scheme. > > For x86_64 we've been fearing the performance of that 2k copy, but I > don't think we've ever actually bit the bullet and implemented it to see > how bad it really is. I actually did at some point and depending on the workload the overhead was clearly measurable. And yes, it fell apart with PCID and I could not come up with a scheme for it which did not suck horribly. So I burried the patches in the poison cabinet. Aside of that, we'd need to implement that for a eight other architectures as well... 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 Wed, Sep 23 2020 at 10:40, peterz wrote: > Right, so I'm concerned. migrate_disable() wrecks pretty much all > Real-Time scheduler theory we have, and PREEMPRT_RT bringing it in is > somewhat ironic. It's even more ironic that the approach of PREEMPT_RT has been 'pragmatic ignorance of theory' from the very beginning and despite violating all theories it still works. :) > Yes, it allows breaking up non-preemptible regions of non-deterministic > duration, and thereby both reduce and bound the scheduling latency, the > cost for doing that is that the theory on CPU utilization/bandwidth go > out the window. I agree, that the theory goes out of the window, but does it matter in practice? I've yet to see a report of migrate disable stacking being the culprit of a missed deadline and I surely have stared at lots of reports in the past 10+ years. > To easily see this consider an SMP system with a number of tasks equal > to the number of CPUs. On a regular (preempt_disable) kernel we can > always run each task, by virtue of always having an idle CPU to take the > task. > > 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. > > The end result is that, like with unbounded latency, we will still miss > our deadline, simply because we got starved for CPU. I'm well aware of that. > Now, while we could (with a _lot_ of work) rework the kernel to not rely > on the implicit per-cpu ness of things like spinlock_t, the moment we > bring in basic primitives that rely on migrate_disable() we're stuck > with it. Right, but we are stuck with per CPUness and distangling that is just infeasible IMO. > The problem is; afaict there's been no research into this problem. There is no research on a lot of things the kernel does :) > There might be scheduling (read: balancing) schemes that can > mitigate/solve this problem, or it might prove to be a 'hard' problem, > I just don't know. In practive balancing surely can take the number of preempted tasks which are in a migrate disable section into account which would be just another measure to work around the fact that the kernel is not adhering to the theories. It never did that even w/o migrate disable. > But once we start down this road, it's going to be hell to get rid of > it. Like most of the other things the kernel came up with to deal with the oddities of modern hardware :) > That's why I've been arguing (for many years) to strictly limit this to > PREEMPT_RT and only as a gap-stop, not a fundamental primitive to build > on. I know, but short of rewriting the world, I'm not seing the faintest plan to remove the stop gap. :) As we discussed not long ago we have too many inconsistent preemption models already. RT is adding yet another one. And that's worse than introducing migrate disable as a general available facility. IMO, reaching a point of consistency where our different preemption models (NONE, VOLUNTARY, PREEMPT. RT) build on each other is far more important. For !RT migrate disable is far less of an danger than for RT kernels because the amount of code which will use it is rather limited compared to code which still will disable preemption implicit through spin and rw locks. On RT converting these locks to 'sleepable spinlocks' is just possible because RT forces almost everything into task context and migrate disable is just the obvious decomposition of preempt disable which implicitely disables migration. But that means that RT is by orders of magnitude more prone to run into the scheduling trainwreck you are worried about. It just refuses to do so at least with real world work loads. I'm surely in favour of having solid theories behind implementation, but at some point you just have to bite the bullet and chose pragmatism in order to make progress. Proliferating inconsistency is not real progress, as it is violating the most fundamental engineering principles. That's by far more dangerous than violating scheduling theories which are built on perfect models and therefore enforce violation by practical implementations anyway. 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 Mon, Sep 21 2020 at 21:27, Thomas Gleixner wrote: > On Mon, Sep 21 2020 at 09:24, Linus Torvalds wrote: >> On Mon, Sep 21, 2020 at 12:39 AM Thomas Gleixner wrote: >> Maybe we really *could* call this new kmap functionality something >> like "kmap_percpu()" (or maybe "local" is good enough), and make it >> act like your RT code does for spinlocks - not disable preemption, but >> only disabling CPU migration. > > I"m all for it, but the scheduler people have opinions :) I just took the latest version of migrate disable patches https://lore.kernel.org/r/20200921163557.234036...@infradead.org removed the RT dependency on top of them and adopted the kmap stuff (addressing the various comments while at it and unbreaking ARM). I'm not going to post that until there is consensus about the general availability of migrate disable, but for those who want to play with it I've pushed the resulting combo out to: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git highmem For testing I've tweaked a few places to use the new _local() variants and it survived testing so far and I've verified that there is actual preemption which means zap/restore of the thread local kmaps. 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 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? I mean make it a priority inheritance PER CPU lock. That is, no two tasks could do a migrate_disable() on the same CPU? If one task does a migrate_disable() and then gets preempted and the preempting task does a migrate_disable() on the same CPU, it will block and wait for the first task to do a migrate_enable(). No two tasks on the same CPU could enter the migrate_disable() section simultaneously, just like no two tasks could enter a preempt_disable() section. In essence, we just allow local_lock() to be used for both RT and !RT. Perhaps make migrate_disable() an anonymous local_lock()? This should lower the SHC in theory, if you can't have stacked migrate disables 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 v5 4/6] dt-bindings: dw-apb-ictl: support hierarchy irq domain
On Fri, 18 Sep 2020 19:22:00 +0800, Zhen Lei wrote: > Add support to use dw-apb-ictl as primary interrupt controller. > > Signed-off-by: Zhen Lei > --- > .../bindings/interrupt-controller/snps,dw-apb-ictl.txt | 14 > +- > 1 file changed, 13 insertions(+), 1 deletion(-) > Reviewed-by: Rob Herring ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v5 5/6] dt-bindings: dw-apb-ictl: convert to json-schema
On Fri, Sep 18, 2020 at 07:22:01PM +0800, Zhen Lei wrote:
> 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 | 75
> ++
> 2 files changed, 75 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 = <&gic>;
> - 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..70c12979c843bf0
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
> @@ -0,0 +1,75 @@
> +# 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:
> + - Marc Zyngier
Usually this would be an owner for this IP block, not the subsystem
maintainer.
> +
> +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.
> +
> +allOf:
> + - $ref: /schemas/interrupt-controller.yaml#
You can drop this, it's already applied based on node name.
> +
> +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.
Need to define how many reg regions (maxItems: 1).
> +
> + interrupts:
> +description:
> + 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.
> +minItems: 1
> +maxItems: 65
65 connections to the primary interrupt controller? I think this is for
the child interrupts? If so, move to #interrupt
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Wed, Sep 23 2020 at 11:52, 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? > > I mean make it a priority inheritance PER CPU lock. > > That is, no two tasks could do a migrate_disable() on the same CPU? If > one task does a migrate_disable() and then gets preempted and the > preempting task does a migrate_disable() on the same CPU, it will block > and wait for the first task to do a migrate_enable(). > > No two tasks on the same CPU could enter the migrate_disable() section > simultaneously, just like no two tasks could enter a preempt_disable() > section. > > In essence, we just allow local_lock() to be used for both RT and !RT. > > Perhaps make migrate_disable() an anonymous local_lock()? > > This should lower the SHC in theory, if you can't have stacked migrate > disables on the same CPU. I'm pretty sure this ends up in locking hell pretty fast and aside of that it's not working for scenarios like: kmap_local(); migrate_disable(); ... copy_from_user() -> #PF -> schedule() which brought us into that discussion in the first place. You would stop any other migrate disable user from running until the page fault is resolved... 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 Wed, 23 Sep 2020 22:55:54 +0200 Thomas Gleixner wrote: > > Perhaps make migrate_disable() an anonymous local_lock()? > > > > This should lower the SHC in theory, if you can't have stacked migrate > > disables on the same CPU. > > I'm pretty sure this ends up in locking hell pretty fast and aside of > that it's not working for scenarios like: > > kmap_local(); >migrate_disable(); >... > > copy_from_user() > -> #PF >-> schedule() > > which brought us into that discussion in the first place. You would stop > any other migrate disable user from running until the page fault is > resolved... 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. 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. 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. -- Steve ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v5 5/6] dt-bindings: dw-apb-ictl: convert to json-schema
On 2020/9/24 4:49, Rob Herring wrote:
> On Fri, Sep 18, 2020 at 07:22:01PM +0800, Zhen Lei wrote:
>> 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 | 75
>> ++
>> 2 files changed, 75 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 = <&gic>;
>> -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..70c12979c843bf0
>> --- /dev/null
>> +++
>> b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
>> @@ -0,0 +1,75 @@
>> +# 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:
>> + - Marc Zyngier
>
> Usually this would be an owner for this IP block, not the subsystem
> maintainer.
OK, I will change it to the author of the file "snps,dw-apb-ictl.txt".
>
>> +
>> +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.
>> +
>> +allOf:
>> + - $ref: /schemas/interrupt-controller.yaml#
>
> You can drop this, it's already applied based on node name.
But if we drop this, the "snps,dw-apb-ictl.yaml" can not require that the node
name
must match '^interrupt-controller(@[0-9a-f,]+)*$'. The problem of Patch 6/6 was
discovered by this.
>
>> +
>> +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.
>
> Need to define how many reg regions (maxItems: 1).
OK, I will add it.
>
>> +
>> + interrupts:
>> +description:
>> + Interrupt reference to primary interrupt controller.
>> +
>> + The interrupt sources map to the corresponding
Re: [PATCH v5 5/6] dt-bindings: dw-apb-ictl: convert to json-schema
On 2020/9/24 11:26, Leizhen (ThunderTown) wrote:
>
>
> On 2020/9/24 4:49, Rob Herring wrote:
>> On Fri, Sep 18, 2020 at 07:22:01PM +0800, Zhen Lei wrote:
>>> 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 | 75
>>> ++
>>> 2 files changed, 75 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 = <&gic>;
>>> - 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..70c12979c843bf0
>>> --- /dev/null
>>> +++
>>> b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.yaml
>>> @@ -0,0 +1,75 @@
>>> +# 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:
>>> + - Marc Zyngier
>>
>> Usually this would be an owner for this IP block, not the subsystem
>> maintainer.
>
> OK, I will change it to the author of the file "snps,dw-apb-ictl.txt".
>
>
>>
>>> +
>>> +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.
>>> +
>>> +allOf:
>>> + - $ref: /schemas/interrupt-controller.yaml#
>>
>> You can drop this, it's already applied based on node name.
> But if we drop this, the "snps,dw-apb-ictl.yaml" can not require that the
> node name
> must match '^interrupt-controller(@[0-9a-f,]+)*$'. The problem of Patch 6/6
> was
> discovered by this.
>
>>
>>> +
>>> +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.
>>
>> Need to define how many reg regions (maxItems: 1).
>
> OK, I
ARC: allmodconfig: Error: inappropriate arguments for opcode 'mpyd'
arc:allmodconfig build failed with gcc-8, gcc-9 and gcc-10 on Linus's
mainline tree.
Build log:
make -sk KBUILD_BUILD_USER=TuxBuild -C/linux ARCH=arc
CROSS_COMPILE=arc-elf32- HOSTCC=gcc CC="sccache arc-elf32-gcc" O=build
allmodconfig
make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=arc
CROSS_COMPILE=arc-elf32- HOSTCC=gcc CC="sccache arc-elf32-gcc" O=build
uImage
In file included from :31:
./usr/include/sound/hdspm.h:48:2: error: unknown type name ‘__u32’
__u32 input_peaks[64];
^
./usr/include/sound/hdspm.h:49:2: error: unknown type name ‘__u32’
__u32 playback_peaks[64];
^
./usr/include/sound/hdspm.h:50:2: error: unknown type name ‘__u32’
__u32 output_peaks[64];
^
./usr/include/sound/hdspm.h:52:2: error: unknown type name ‘__u64’
__u64 input_rms[64];
^
./usr/include/sound/hdspm.h:53:2: error: unknown type name ‘__u64’
__u64 playback_rms[64];
^
./usr/include/sound/hdspm.h:54:2: error: unknown type name ‘__u64’
__u64 output_rms[64];
^
./usr/include/sound/hdspm.h:56:2: error: unknown type name ‘__u8’
__u8 speed; /* enum {ss, ds, qs} */
^~~~
./usr/include/sound/hdspm.h:157:2: error: unknown type name ‘__u8’
__u8 card_type; /* enum hdspm_io_type */
^~~~
./usr/include/sound/hdspm.h:160:2: error: unknown type name ‘__u64’
__u64 card_clock;
^
./usr/include/sound/hdspm.h:161:2: error: unknown type name ‘__u32’
__u32 master_period;
^
./usr/include/sound/hdspm.h:165:4: error: unknown type name ‘__u8’
__u8 sync_wc; /* enum hdspm_sync */
^~~~
./usr/include/sound/hdspm.h:166:4: error: unknown type name ‘__u8’
__u8 sync_madi; /* enum hdspm_sync */
^~~~
./usr/include/sound/hdspm.h:167:4: error: unknown type name ‘__u8’
__u8 sync_tco; /* enum hdspm_sync */
^~~~
./usr/include/sound/hdspm.h:168:4: error: unknown type name ‘__u8’
__u8 sync_in; /* enum hdspm_sync */
^~~~
./usr/include/sound/hdspm.h:169:4: error: unknown type name ‘__u8’
__u8 madi_input; /* enum hdspm_madi_input */
^~~~
./usr/include/sound/hdspm.h:170:4: error: unknown type name ‘__u8’
__u8 channel_format; /* enum hdspm_madi_channel_format */
^~~~
./usr/include/sound/hdspm.h:171:4: error: unknown type name ‘__u8’
__u8 frame_format; /* enum hdspm_madi_frame_format */
^~~~
./usr/include/sound/hdspm.h:186:2: error: unknown type name ‘__u8’
__u8 card_type; /* enum hdspm_io_type */
^~~~
make[3]: *** [../usr/include/Makefile:108:
usr/include/sound/hdspm.hdrtest] Error 1
In file included from :31:
./usr/include/sound/hdsp.h:39:2: error: unknown type name ‘__u32’
__u32 input_peaks[26];
^
./usr/include/sound/hdsp.h:40:2: error: unknown type name ‘__u32’
__u32 playback_peaks[26];
^
./usr/include/sound/hdsp.h:41:2: error: unknown type name ‘__u32’
__u32 output_peaks[28];
^
./usr/include/sound/hdsp.h:42:2: error: unknown type name ‘__u64’
__u64 input_rms[26];
^
./usr/include/sound/hdsp.h:43:2: error: unknown type name ‘__u64’
__u64 playback_rms[26];
^
./usr/include/sound/hdsp.h:45:2: error: unknown type name ‘__u64’
__u64 output_rms[26];
^
make[3]: *** [../usr/include/Makefile:108:
usr/include/sound/hdsp.hdrtest] Error 1
{standard input}: Assembler messages:
{standard input}:9360: Error: inappropriate arguments for opcode 'mpyd'
make[3]: *** [../scripts/Makefile.build:283: kernel/sched/core.o] Error 1
../arch/arc/kernel/kgdb.c: In function ‘kgdb_arch_handle_exception’:
../arch/arc/kernel/kgdb.c:141:6: warning: this statement may fall
through [-Wimplicit-fallthrough=]
if (kgdb_hex2long(&ptr, &addr))
^
../arch/arc/kernel/kgdb.c:144:2: note: here
case 'D':
^~~~
In file included from ./usr/include/linux/netfilter_ipv6/ip6_tables.h:21,
from :31:
./usr/include/linux/if.h:28:10: fatal error: sys/socket.h: No such
file or directory
#include/* for struct sockaddr. */
^~
compilation terminated.
make[3]: *** [../usr/include/Makefile:108:
usr/include/linux/netfilter_ipv6/ip6_tables.hdrtest] Error 1
../arch/arc/kernel/perf_event.c: In function ‘arc_pmu_device_probe’:
../arch/arc/kernel/perf_event.c:645:3: warning: ignoring return value
of ‘request_percpu_irq’, declared with attribute warn_unused_result
[-Wunused-result]
request_percpu_irq(irq, arc_pmu_intr, "ARC perf counters",
^~
this_cpu_ptr(&arc_pmu_cpu));
~~~
In file included from ./usr/include/linux/if_bonding.h:47,
from :31:
./usr/include/linux/if.h:28:10: fatal error: sys/socket.h: No such
file or directory
#include/* for struct sockaddr. */
^~
compilation terminated.
make[3]: *** [../usr/include/Makefile:108:
usr/include/linux/if_bonding.hdrtest] Error 1
In file included from ./usr/include/linux/netfilter_ipv4/ip_tables.h:21,
from :31:
./usr/include/linux/if.h:28:10: fatal error: sys/socket.h: No such
file or dir
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
