Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 02/07/2018 05:32 PM, Laura Abbott wrote: > On 02/07/2018 07:10 AM, Alexey Skidanov wrote: >> >> >> On 02/07/2018 04:58 PM, Laura Abbott wrote: >>> On 02/06/2018 11:05 PM, Alexey Skidanov wrote: > Yup, you've hit upon a key problem. Having fallbacks be stable > was always a problem and the recommendation these days is to > not rely on them. You can specify a heap at a time and fallback > manually if you want that behavior. > > If you have a proposal to make fallbacks work reliably without > overly complicating the ABI I'm happy to review it. > > Thanks, > Laura > I think it's possible to "automate" the "manual fallback" behavior. But the real issues is using heap id to specify the particular heap object. Current API (allocation IOCTL) requires to specify the particular heap object by using heap id. From the other hand, the user space doesn't control the heaps creation order and heap id assignment. So it may be tricky, especially when more than one object of the same heap type is created automatically. Thanks, Alexey >>> >>> The query ioctl is designed to get the heap ID information without >>> needing to rely on the linking order or anything else defined in >>> the kernel. >>> >>> Thanks, >>> Laura >> >> That is true. But if we have 2 *automatically created* heaps of the same >> type, how userspace can distinguish between them? >> >> Thanks, >> Alexey >> > > The query ioctl also gives the name which should be different > for each heap. It's not ideal but the name/heap type are the best > way to differentiate between heaps without resorting to hard > coding. > > Thanks, > Laura You are correct ... It will work (assuming that user space developer knows where to look for the name :) ) So, the userspace may pass the list of pairs [heap type, name] (as part of allocation ioctl) defining the fallback order. Thanks, Alexey
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 02/07/2018 05:32 PM, Laura Abbott wrote: > On 02/07/2018 07:10 AM, Alexey Skidanov wrote: >> >> >> On 02/07/2018 04:58 PM, Laura Abbott wrote: >>> On 02/06/2018 11:05 PM, Alexey Skidanov wrote: > Yup, you've hit upon a key problem. Having fallbacks be stable > was always a problem and the recommendation these days is to > not rely on them. You can specify a heap at a time and fallback > manually if you want that behavior. > > If you have a proposal to make fallbacks work reliably without > overly complicating the ABI I'm happy to review it. > > Thanks, > Laura > I think it's possible to "automate" the "manual fallback" behavior. But the real issues is using heap id to specify the particular heap object. Current API (allocation IOCTL) requires to specify the particular heap object by using heap id. From the other hand, the user space doesn't control the heaps creation order and heap id assignment. So it may be tricky, especially when more than one object of the same heap type is created automatically. Thanks, Alexey >>> >>> The query ioctl is designed to get the heap ID information without >>> needing to rely on the linking order or anything else defined in >>> the kernel. >>> >>> Thanks, >>> Laura >> >> That is true. But if we have 2 *automatically created* heaps of the same >> type, how userspace can distinguish between them? >> >> Thanks, >> Alexey >> > > The query ioctl also gives the name which should be different > for each heap. It's not ideal but the name/heap type are the best > way to differentiate between heaps without resorting to hard > coding. > > Thanks, > Laura You are correct ... It will work (assuming that user space developer knows where to look for the name :) ) So, the userspace may pass the list of pairs [heap type, name] (as part of allocation ioctl) defining the fallback order. Thanks, Alexey
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 02/07/2018 07:10 AM, Alexey Skidanov wrote: On 02/07/2018 04:58 PM, Laura Abbott wrote: On 02/06/2018 11:05 PM, Alexey Skidanov wrote: Yup, you've hit upon a key problem. Having fallbacks be stable was always a problem and the recommendation these days is to not rely on them. You can specify a heap at a time and fallback manually if you want that behavior. If you have a proposal to make fallbacks work reliably without overly complicating the ABI I'm happy to review it. Thanks, Laura I think it's possible to "automate" the "manual fallback" behavior. But the real issues is using heap id to specify the particular heap object. Current API (allocation IOCTL) requires to specify the particular heap object by using heap id. From the other hand, the user space doesn't control the heaps creation order and heap id assignment. So it may be tricky, especially when more than one object of the same heap type is created automatically. Thanks, Alexey The query ioctl is designed to get the heap ID information without needing to rely on the linking order or anything else defined in the kernel. Thanks, Laura That is true. But if we have 2 *automatically created* heaps of the same type, how userspace can distinguish between them? Thanks, Alexey The query ioctl also gives the name which should be different for each heap. It's not ideal but the name/heap type are the best way to differentiate between heaps without resorting to hard coding. Thanks, Laura
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 02/07/2018 07:10 AM, Alexey Skidanov wrote: On 02/07/2018 04:58 PM, Laura Abbott wrote: On 02/06/2018 11:05 PM, Alexey Skidanov wrote: Yup, you've hit upon a key problem. Having fallbacks be stable was always a problem and the recommendation these days is to not rely on them. You can specify a heap at a time and fallback manually if you want that behavior. If you have a proposal to make fallbacks work reliably without overly complicating the ABI I'm happy to review it. Thanks, Laura I think it's possible to "automate" the "manual fallback" behavior. But the real issues is using heap id to specify the particular heap object. Current API (allocation IOCTL) requires to specify the particular heap object by using heap id. From the other hand, the user space doesn't control the heaps creation order and heap id assignment. So it may be tricky, especially when more than one object of the same heap type is created automatically. Thanks, Alexey The query ioctl is designed to get the heap ID information without needing to rely on the linking order or anything else defined in the kernel. Thanks, Laura That is true. But if we have 2 *automatically created* heaps of the same type, how userspace can distinguish between them? Thanks, Alexey The query ioctl also gives the name which should be different for each heap. It's not ideal but the name/heap type are the best way to differentiate between heaps without resorting to hard coding. Thanks, Laura
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 02/07/2018 04:58 PM, Laura Abbott wrote: > On 02/06/2018 11:05 PM, Alexey Skidanov wrote: >> >> >>> Yup, you've hit upon a key problem. Having fallbacks be stable >>> was always a problem and the recommendation these days is to >>> not rely on them. You can specify a heap at a time and fallback >>> manually if you want that behavior. >>> >>> If you have a proposal to make fallbacks work reliably without >>> overly complicating the ABI I'm happy to review it. >>> >>> Thanks, >>> Laura >>> >> I think it's possible to "automate" the "manual fallback" behavior. But >> the real issues is using heap id to specify the particular heap object. >> >> Current API (allocation IOCTL) requires to specify the particular heap >> object by using heap id. From the other hand, the user space doesn't >> control the heaps creation order and heap id assignment. So it may be >> tricky, especially when more than one object of the same heap type is >> created automatically. >> >> Thanks, >> Alexey >> >> > > The query ioctl is designed to get the heap ID information without > needing to rely on the linking order or anything else defined in > the kernel. > > Thanks, > Laura That is true. But if we have 2 *automatically created* heaps of the same type, how userspace can distinguish between them? Thanks, Alexey
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 02/07/2018 04:58 PM, Laura Abbott wrote: > On 02/06/2018 11:05 PM, Alexey Skidanov wrote: >> >> >>> Yup, you've hit upon a key problem. Having fallbacks be stable >>> was always a problem and the recommendation these days is to >>> not rely on them. You can specify a heap at a time and fallback >>> manually if you want that behavior. >>> >>> If you have a proposal to make fallbacks work reliably without >>> overly complicating the ABI I'm happy to review it. >>> >>> Thanks, >>> Laura >>> >> I think it's possible to "automate" the "manual fallback" behavior. But >> the real issues is using heap id to specify the particular heap object. >> >> Current API (allocation IOCTL) requires to specify the particular heap >> object by using heap id. From the other hand, the user space doesn't >> control the heaps creation order and heap id assignment. So it may be >> tricky, especially when more than one object of the same heap type is >> created automatically. >> >> Thanks, >> Alexey >> >> > > The query ioctl is designed to get the heap ID information without > needing to rely on the linking order or anything else defined in > the kernel. > > Thanks, > Laura That is true. But if we have 2 *automatically created* heaps of the same type, how userspace can distinguish between them? Thanks, Alexey
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 02/06/2018 11:05 PM, Alexey Skidanov wrote: Yup, you've hit upon a key problem. Having fallbacks be stable was always a problem and the recommendation these days is to not rely on them. You can specify a heap at a time and fallback manually if you want that behavior. If you have a proposal to make fallbacks work reliably without overly complicating the ABI I'm happy to review it. Thanks, Laura I think it's possible to "automate" the "manual fallback" behavior. But the real issues is using heap id to specify the particular heap object. Current API (allocation IOCTL) requires to specify the particular heap object by using heap id. From the other hand, the user space doesn't control the heaps creation order and heap id assignment. So it may be tricky, especially when more than one object of the same heap type is created automatically. Thanks, Alexey The query ioctl is designed to get the heap ID information without needing to rely on the linking order or anything else defined in the kernel. Thanks, Laura
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 02/06/2018 11:05 PM, Alexey Skidanov wrote: Yup, you've hit upon a key problem. Having fallbacks be stable was always a problem and the recommendation these days is to not rely on them. You can specify a heap at a time and fallback manually if you want that behavior. If you have a proposal to make fallbacks work reliably without overly complicating the ABI I'm happy to review it. Thanks, Laura I think it's possible to "automate" the "manual fallback" behavior. But the real issues is using heap id to specify the particular heap object. Current API (allocation IOCTL) requires to specify the particular heap object by using heap id. From the other hand, the user space doesn't control the heaps creation order and heap id assignment. So it may be tricky, especially when more than one object of the same heap type is created automatically. Thanks, Alexey The query ioctl is designed to get the heap ID information without needing to rely on the linking order or anything else defined in the kernel. Thanks, Laura
Re: staging: ion: ION allocation fall back order depends on heap linkage order
> Yup, you've hit upon a key problem. Having fallbacks be stable > was always a problem and the recommendation these days is to > not rely on them. You can specify a heap at a time and fallback > manually if you want that behavior. > > If you have a proposal to make fallbacks work reliably without > overly complicating the ABI I'm happy to review it. > > Thanks, > Laura > I think it's possible to "automate" the "manual fallback" behavior. But the real issues is using heap id to specify the particular heap object. Current API (allocation IOCTL) requires to specify the particular heap object by using heap id. From the other hand, the user space doesn't control the heaps creation order and heap id assignment. So it may be tricky, especially when more than one object of the same heap type is created automatically. Thanks, Alexey
Re: staging: ion: ION allocation fall back order depends on heap linkage order
> Yup, you've hit upon a key problem. Having fallbacks be stable > was always a problem and the recommendation these days is to > not rely on them. You can specify a heap at a time and fallback > manually if you want that behavior. > > If you have a proposal to make fallbacks work reliably without > overly complicating the ABI I'm happy to review it. > > Thanks, > Laura > I think it's possible to "automate" the "manual fallback" behavior. But the real issues is using heap id to specify the particular heap object. Current API (allocation IOCTL) requires to specify the particular heap object by using heap id. From the other hand, the user space doesn't control the heaps creation order and heap id assignment. So it may be tricky, especially when more than one object of the same heap type is created automatically. Thanks, Alexey
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 01/28/2018 08:24 AM, Alexey Skidanov wrote: Hi, According to my understanding, the allocation fall back order completely depends on heap->id that is assigned during the heap creation: plist_for_each_entry(heap, >heaps, node) { /* if the caller didn't specify this heap id */ if (!((1 << heap->id) & heap_id_mask)) continue; buffer = ion_buffer_create(heap, dev, len, flags); if (!IS_ERR(buffer)) break; } On creation, each heap is added to the priority list according to the priority assigned: ... static int heap_id; ... void ion_device_add_heap(struct ion_heap *heap) { ... heap->id = heap_id++; ... } The order of creation is the order of linkage defined in the Makefile. Thus, by default, we have: heap id 2, type ION_HEAP_TYPE_DMA heap id 1, type ION_HEAP_TYPE_SYSTEM heap id 0, type ION_HEAP_TYPE_SYSTEM_CONTIG Changing the linkage order: diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile index bb30bf8..e05052c 100644 --- a/drivers/staging/android/ion/Makefile +++ b/drivers/staging/android/ion/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o -obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o +obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o I get the following order: heap id 2, type ION_HEAP_TYPE_SYSTEM heap id 1, type ION_HEAP_TYPE_SYSTEM_CONTIG heap id 0, type ION_HEAP_TYPE_DMA So, if the user specifies more than 1 heap in the heap_id_mask during allocation, the allocation fall back order completely depends on the order of linkage. Probably, it's better to let the user to define the fall back order (and NOT to be dependent on the linkage order at all) ? Yup, you've hit upon a key problem. Having fallbacks be stable was always a problem and the recommendation these days is to not rely on them. You can specify a heap at a time and fallback manually if you want that behavior. If you have a proposal to make fallbacks work reliably without overly complicating the ABI I'm happy to review it. Thanks, Laura
Re: staging: ion: ION allocation fall back order depends on heap linkage order
On 01/28/2018 08:24 AM, Alexey Skidanov wrote: Hi, According to my understanding, the allocation fall back order completely depends on heap->id that is assigned during the heap creation: plist_for_each_entry(heap, >heaps, node) { /* if the caller didn't specify this heap id */ if (!((1 << heap->id) & heap_id_mask)) continue; buffer = ion_buffer_create(heap, dev, len, flags); if (!IS_ERR(buffer)) break; } On creation, each heap is added to the priority list according to the priority assigned: ... static int heap_id; ... void ion_device_add_heap(struct ion_heap *heap) { ... heap->id = heap_id++; ... } The order of creation is the order of linkage defined in the Makefile. Thus, by default, we have: heap id 2, type ION_HEAP_TYPE_DMA heap id 1, type ION_HEAP_TYPE_SYSTEM heap id 0, type ION_HEAP_TYPE_SYSTEM_CONTIG Changing the linkage order: diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile index bb30bf8..e05052c 100644 --- a/drivers/staging/android/ion/Makefile +++ b/drivers/staging/android/ion/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o -obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o +obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o I get the following order: heap id 2, type ION_HEAP_TYPE_SYSTEM heap id 1, type ION_HEAP_TYPE_SYSTEM_CONTIG heap id 0, type ION_HEAP_TYPE_DMA So, if the user specifies more than 1 heap in the heap_id_mask during allocation, the allocation fall back order completely depends on the order of linkage. Probably, it's better to let the user to define the fall back order (and NOT to be dependent on the linkage order at all) ? Yup, you've hit upon a key problem. Having fallbacks be stable was always a problem and the recommendation these days is to not rely on them. You can specify a heap at a time and fallback manually if you want that behavior. If you have a proposal to make fallbacks work reliably without overly complicating the ABI I'm happy to review it. Thanks, Laura
staging: ion: ION allocation fall back order depends on heap linkage order
Hi, According to my understanding, the allocation fall back order completely depends on heap->id that is assigned during the heap creation: plist_for_each_entry(heap, >heaps, node) { /* if the caller didn't specify this heap id */ if (!((1 << heap->id) & heap_id_mask)) continue; buffer = ion_buffer_create(heap, dev, len, flags); if (!IS_ERR(buffer)) break; } On creation, each heap is added to the priority list according to the priority assigned: ... static int heap_id; ... void ion_device_add_heap(struct ion_heap *heap) { ... heap->id = heap_id++; ... } The order of creation is the order of linkage defined in the Makefile. Thus, by default, we have: heap id 2, type ION_HEAP_TYPE_DMA heap id 1, type ION_HEAP_TYPE_SYSTEM heap id 0, type ION_HEAP_TYPE_SYSTEM_CONTIG Changing the linkage order: diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile index bb30bf8..e05052c 100644 --- a/drivers/staging/android/ion/Makefile +++ b/drivers/staging/android/ion/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o -obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o +obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o I get the following order: heap id 2, type ION_HEAP_TYPE_SYSTEM heap id 1, type ION_HEAP_TYPE_SYSTEM_CONTIG heap id 0, type ION_HEAP_TYPE_DMA So, if the user specifies more than 1 heap in the heap_id_mask during allocation, the allocation fall back order completely depends on the order of linkage. Probably, it's better to let the user to define the fall back order (and NOT to be dependent on the linkage order at all) ? Thanks, Alexey
staging: ion: ION allocation fall back order depends on heap linkage order
Hi, According to my understanding, the allocation fall back order completely depends on heap->id that is assigned during the heap creation: plist_for_each_entry(heap, >heaps, node) { /* if the caller didn't specify this heap id */ if (!((1 << heap->id) & heap_id_mask)) continue; buffer = ion_buffer_create(heap, dev, len, flags); if (!IS_ERR(buffer)) break; } On creation, each heap is added to the priority list according to the priority assigned: ... static int heap_id; ... void ion_device_add_heap(struct ion_heap *heap) { ... heap->id = heap_id++; ... } The order of creation is the order of linkage defined in the Makefile. Thus, by default, we have: heap id 2, type ION_HEAP_TYPE_DMA heap id 1, type ION_HEAP_TYPE_SYSTEM heap id 0, type ION_HEAP_TYPE_SYSTEM_CONTIG Changing the linkage order: diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile index bb30bf8..e05052c 100644 --- a/drivers/staging/android/ion/Makefile +++ b/drivers/staging/android/ion/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o -obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o +obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o I get the following order: heap id 2, type ION_HEAP_TYPE_SYSTEM heap id 1, type ION_HEAP_TYPE_SYSTEM_CONTIG heap id 0, type ION_HEAP_TYPE_DMA So, if the user specifies more than 1 heap in the heap_id_mask during allocation, the allocation fall back order completely depends on the order of linkage. Probably, it's better to let the user to define the fall back order (and NOT to be dependent on the linkage order at all) ? Thanks, Alexey