| Message ID | 20230510-dt-resv-bottom-up-v1-1-3bf68873dbed@gerhold.net |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | of: reserved_mem: Provide more control about allocation behavior | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On Mon, May 15, 2023 at 12:12:16PM +0200, Stephan Gerhold wrote: > Right now the allocation behavior for dynamic reserved memory is > implementation-defined. On Linux it is dependent on the architecture. > This is usually fine if the address is completely arbitrary. > > However, when using "alloc-ranges" it is helpful to allow controlling > this. That way you can make sure that the reservations are placed next > to other (static) allocations to keep the free memory contiguous if > possible. That should already be possible with all the information you already have. IOW, you are looking at all the region and "alloc-ranges" addresses to decide top-down or bottom-up. Why can't the kernel do that. Alternatively, if you really care about the allocation locations, don't use dynamic regions. > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > .../bindings/reserved-memory/reserved-memory.yaml | 39 ++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > index c680e397cfd2..56f4bc6137e7 100644 > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > @@ -52,6 +52,18 @@ properties: > Address and Length pairs. Specifies regions of memory that are > acceptable to allocate from. > > + alloc-bottom-up: > + type: boolean > + description: > > + Specifies that the memory region should be preferably allocated > + at the lowest available address within the "alloc-ranges" region. > + > + alloc-top-down: > + type: boolean > + description: > > + Specifies that the memory region should be preferably allocated > + at the highest available address within the "alloc-ranges" region. What happens when both are set? > + > iommu-addresses: > $ref: /schemas/types.yaml#/definitions/phandle-array > description: > > @@ -93,6 +105,10 @@ properties: > system can use that region to store volatile or cached data that > can be otherwise regenerated or migrated elsewhere. > > +dependencies: > + alloc-bottom-up: [alloc-ranges] > + alloc-top-down: [alloc-ranges] > + > allOf: > - if: > required: > @@ -178,4 +194,27 @@ examples: > }; > }; > }; > + > + - | > + / { > + compatible = "foo"; > + model = "foo"; > + > + #address-cells = <2>; > + #size-cells = <2>; > + > + reserved-memory { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + adsp_mem: adsp { > + size = <0x0 0x600000>; > + alignment = <0x0 0x100000>; > + alloc-ranges = <0x0 0x86800000 0x0 0x10000000>; > + alloc-bottom-up; > + no-map; > + }; > + }; > + }; > ... > > -- > 2.40.1 >
Hi Rob, Thanks for your suggestions! On Thu, Jun 08, 2023 at 08:02:56AM -0600, Rob Herring wrote: > On Mon, May 15, 2023 at 12:12:16PM +0200, Stephan Gerhold wrote: > > Right now the allocation behavior for dynamic reserved memory is > > implementation-defined. On Linux it is dependent on the architecture. > > This is usually fine if the address is completely arbitrary. > > > > However, when using "alloc-ranges" it is helpful to allow controlling > > this. That way you can make sure that the reservations are placed next > > to other (static) allocations to keep the free memory contiguous if > > possible. > > That should already be possible with all the information you > already have. IOW, you are looking at all the region and "alloc-ranges" > addresses to decide top-down or bottom-up. Why can't the kernel do that. > Would you accept a patch implementing such a behavior? There are obviously infinitely complicated algorithms possible for the allocation. A fairly simple one would be to check if the "alloc-ranges" overlap or are adjacent to an already existing reservation, i.e. 1. If the "alloc-range" starts at the end or inside an existing reservation, use bottom-up. 2. If the "alloc-range" ends at the start or inside an existing reservation, use top-down. 3. If both or none is the case, keep current (implementation-defined) behavior. For reference, here are some examples how it behaves. |...| is the unallocated memory, RRR existing allocations, and each RRR--- line below a requested alloc-range (and where it was allocated): Bottom-up (rule 1): |.....RRRR................RRRRRRRRR...........| RRR---- ---RRR------- Top-down (rule 2): |.....RRRR................RRRRRRRRR...........| ----RRR ---------RRR------ Otherwise rule 3 just behaves as currently where either bottom-up or top-down is used depending on the implementation/architecture: |.....RRRR................RRRRRRRRR...........| -----RRR or RRR----- ---------------RRR---- or --RRR----------------- There are plenty of edge cases where it doesn't produce the optimal result, but it just results in exactly the same behavior as currently so it's not any worse (with rule 3): |.....RRRR................RRRRRRRRR...........| -----------RRR----- or ----------------RRR ---------------------RRR (no way to handle this or RRR--------------------- with top-down/bottom-up) > Alternatively, if you really care about the allocation locations, don't > use dynamic regions. > Yes, this is the option used at the moment. As outlined in detail in the examples of RFC PATCH 4/5 and 5/5 I would like a solution inbetween. The exact address doesn't matter but the way (direction) the region is filled should preferably stay the same. > > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > > --- > > .../bindings/reserved-memory/reserved-memory.yaml | 39 ++++++++++++++++++++++ > > 1 file changed, 39 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > > index c680e397cfd2..56f4bc6137e7 100644 > > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > > @@ -52,6 +52,18 @@ properties: > > Address and Length pairs. Specifies regions of memory that are > > acceptable to allocate from. > > > > + alloc-bottom-up: > > + type: boolean > > + description: > > > + Specifies that the memory region should be preferably allocated > > + at the lowest available address within the "alloc-ranges" region. > > + > > + alloc-top-down: > > + type: boolean > > + description: > > > + Specifies that the memory region should be preferably allocated > > + at the highest available address within the "alloc-ranges" region. > > What happens when both are set? > They are not meant to be both set. I should have added an if statement for this, sorry about that. Thanks, Stephan
On Fri, Jun 09, 2023 at 11:16:01AM +0200, Stephan Gerhold wrote: > Hi Rob, > > Thanks for your suggestions! > > On Thu, Jun 08, 2023 at 08:02:56AM -0600, Rob Herring wrote: > > On Mon, May 15, 2023 at 12:12:16PM +0200, Stephan Gerhold wrote: > > > Right now the allocation behavior for dynamic reserved memory is > > > implementation-defined. On Linux it is dependent on the architecture. > > > This is usually fine if the address is completely arbitrary. > > > > > > However, when using "alloc-ranges" it is helpful to allow controlling > > > this. That way you can make sure that the reservations are placed next > > > to other (static) allocations to keep the free memory contiguous if > > > possible. > > > > That should already be possible with all the information you > > already have. IOW, you are looking at all the region and "alloc-ranges" > > addresses to decide top-down or bottom-up. Why can't the kernel do that. > > > > Would you accept a patch implementing such a behavior? Yes. > There are obviously infinitely complicated algorithms possible for the > allocation. A fairly simple one would be to check if the "alloc-ranges" > overlap or are adjacent to an already existing reservation, i.e. > > 1. If the "alloc-range" starts at the end or inside an existing > reservation, use bottom-up. > 2. If the "alloc-range" ends at the start or inside an existing > reservation, use top-down. > 3. If both or none is the case, keep current (implementation-defined) > behavior. > > For reference, here are some examples how it behaves. |...| is the > unallocated memory, RRR existing allocations, and each RRR--- line > below a requested alloc-range (and where it was allocated): > > Bottom-up (rule 1): > |.....RRRR................RRRRRRRRR...........| > RRR---- > ---RRR------- > > Top-down (rule 2): > |.....RRRR................RRRRRRRRR...........| > ----RRR > ---------RRR------ > > Otherwise rule 3 just behaves as currently where either bottom-up > or top-down is used depending on the implementation/architecture: > |.....RRRR................RRRRRRRRR...........| > -----RRR > or RRR----- > ---------------RRR---- > or --RRR----------------- > > There are plenty of edge cases where it doesn't produce the optimal > result, but it just results in exactly the same behavior as currently > so it's not any worse (with rule 3): > > |.....RRRR................RRRRRRRRR...........| > -----------RRR----- > or ----------------RRR > ---------------------RRR (no way to handle this > or RRR--------------------- with top-down/bottom-up) > > > Alternatively, if you really care about the allocation locations, don't > > use dynamic regions. > > > > Yes, this is the option used at the moment. As outlined in detail in the > examples of RFC PATCH 4/5 and 5/5 I would like a solution inbetween. The > exact address doesn't matter but the way (direction) the region is > filled should preferably stay the same. > > > > > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > > > --- > > > .../bindings/reserved-memory/reserved-memory.yaml | 39 ++++++++++++++++++++++ > > > 1 file changed, 39 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > > > index c680e397cfd2..56f4bc6137e7 100644 > > > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > > > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > > > @@ -52,6 +52,18 @@ properties: > > > Address and Length pairs. Specifies regions of memory that are > > > acceptable to allocate from. > > > > > > + alloc-bottom-up: > > > + type: boolean > > > + description: > > > > + Specifies that the memory region should be preferably allocated > > > + at the lowest available address within the "alloc-ranges" region. > > > + > > > + alloc-top-down: > > > + type: boolean > > > + description: > > > > + Specifies that the memory region should be preferably allocated > > > + at the highest available address within the "alloc-ranges" region. > > > > What happens when both are set? > > > > They are not meant to be both set. I should have added an if statement > for this, sorry about that. Ideally, you define the properties in a way to avoid that situation rather than relying on schema checks. For example, a single property with values defined for top-down and bottom-up. Rob
diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml index c680e397cfd2..56f4bc6137e7 100644 --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml @@ -52,6 +52,18 @@ properties: Address and Length pairs. Specifies regions of memory that are acceptable to allocate from. + alloc-bottom-up: + type: boolean + description: > + Specifies that the memory region should be preferably allocated + at the lowest available address within the "alloc-ranges" region. + + alloc-top-down: + type: boolean + description: > + Specifies that the memory region should be preferably allocated + at the highest available address within the "alloc-ranges" region. + iommu-addresses: $ref: /schemas/types.yaml#/definitions/phandle-array description: > @@ -93,6 +105,10 @@ properties: system can use that region to store volatile or cached data that can be otherwise regenerated or migrated elsewhere. +dependencies: + alloc-bottom-up: [alloc-ranges] + alloc-top-down: [alloc-ranges] + allOf: - if: required: @@ -178,4 +194,27 @@ examples: }; }; }; + + - | + / { + compatible = "foo"; + model = "foo"; + + #address-cells = <2>; + #size-cells = <2>; + + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + adsp_mem: adsp { + size = <0x0 0x600000>; + alignment = <0x0 0x100000>; + alloc-ranges = <0x0 0x86800000 0x0 0x10000000>; + alloc-bottom-up; + no-map; + }; + }; + }; ...
Right now the allocation behavior for dynamic reserved memory is implementation-defined. On Linux it is dependent on the architecture. This is usually fine if the address is completely arbitrary. However, when using "alloc-ranges" it is helpful to allow controlling this. That way you can make sure that the reservations are placed next to other (static) allocations to keep the free memory contiguous if possible. Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- .../bindings/reserved-memory/reserved-memory.yaml | 39 ++++++++++++++++++++++ 1 file changed, 39 insertions(+)