[REPOST PATCH v4] iommu: Fix potential use-after-free during probe
Kasan has reported the following use after free on dev->iommu.
when a device probe fails and it is in process of freeing dev->iommu
in dev_iommu_free function, a deferred_probe_work_func runs in parallel
and tries to access dev->iommu->fwspec in of_iommu_configure path thus
causing use after free.
BUG: KASAN: use-after-free in of_iommu_configure+0xb4/0x4a4
Read of size 8 at addr ff87a2f1acb8 by task kworker/u16:2/153
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace+0x0/0x33c
show_stack+0x18/0x24
dump_stack_lvl+0x16c/0x1e0
print_address_description+0x84/0x39c
__kasan_report+0x184/0x308
kasan_report+0x50/0x78
__asan_load8+0xc0/0xc4
of_iommu_configure+0xb4/0x4a4
of_dma_configure_id+0x2fc/0x4d4
platform_dma_configure+0x40/0x5c
really_probe+0x1b4/0xb74
driver_probe_device+0x11c/0x228
__device_attach_driver+0x14c/0x304
bus_for_each_drv+0x124/0x1b0
__device_attach+0x25c/0x334
device_initial_probe+0x24/0x34
bus_probe_device+0x78/0x134
deferred_probe_work_func+0x130/0x1a8
process_one_work+0x4c8/0x970
worker_thread+0x5c8/0xaec
kthread+0x1f8/0x220
ret_from_fork+0x10/0x18
Allocated by task 1:
kasan_kmalloc+0xd4/0x114
__kasan_kmalloc+0x10/0x1c
kmem_cache_alloc_trace+0xe4/0x3d4
__iommu_probe_device+0x90/0x394
probe_iommu_group+0x70/0x9c
bus_for_each_dev+0x11c/0x19c
bus_iommu_probe+0xb8/0x7d4
bus_set_iommu+0xcc/0x13c
arm_smmu_bus_init+0x44/0x130 [arm_smmu]
arm_smmu_device_probe+0xb88/0xc54 [arm_smmu]
platform_drv_probe+0xe4/0x13c
really_probe+0x2c8/0xb74
driver_probe_device+0x11c/0x228
device_driver_attach+0xf0/0x16c
__driver_attach+0x80/0x320
bus_for_each_dev+0x11c/0x19c
driver_attach+0x38/0x48
bus_add_driver+0x1dc/0x3a4
driver_register+0x18c/0x244
__platform_driver_register+0x88/0x9c
init_module+0x64/0xff4 [arm_smmu]
do_one_initcall+0x17c/0x2f0
do_init_module+0xe8/0x378
load_module+0x3f80/0x4a40
__se_sys_finit_module+0x1a0/0x1e4
__arm64_sys_finit_module+0x44/0x58
el0_svc_common+0x100/0x264
do_el0_svc+0x38/0xa4
el0_svc+0x20/0x30
el0_sync_handler+0x68/0xac
el0_sync+0x160/0x180
Freed by task 1:
kasan_set_track+0x4c/0x84
kasan_set_free_info+0x28/0x4c
kasan_slab_free+0x120/0x15c
__kasan_slab_free+0x18/0x28
slab_free_freelist_hook+0x204/0x2fc
kfree+0xfc/0x3a4
__iommu_probe_device+0x284/0x394
probe_iommu_group+0x70/0x9c
bus_for_each_dev+0x11c/0x19c
bus_iommu_probe+0xb8/0x7d4
bus_set_iommu+0xcc/0x13c
arm_smmu_bus_init+0x44/0x130 [arm_smmu]
arm_smmu_device_probe+0xb88/0xc54 [arm_smmu]
platform_drv_probe+0xe4/0x13c
really_probe+0x2c8/0xb74
driver_probe_device+0x11c/0x228
device_driver_attach+0xf0/0x16c
__driver_attach+0x80/0x320
bus_for_each_dev+0x11c/0x19c
driver_attach+0x38/0x48
bus_add_driver+0x1dc/0x3a4
driver_register+0x18c/0x244
__platform_driver_register+0x88/0x9c
init_module+0x64/0xff4 [arm_smmu]
do_one_initcall+0x17c/0x2f0
do_init_module+0xe8/0x378
load_module+0x3f80/0x4a40
__se_sys_finit_module+0x1a0/0x1e4
__arm64_sys_finit_module+0x44/0x58
el0_svc_common+0x100/0x264
do_el0_svc+0x38/0xa4
el0_svc+0x20/0x30
el0_sync_handler+0x68/0xac
el0_sync+0x160/0x180
Fix this by setting dev->iommu to NULL first and
then freeing dev_iommu structure in dev_iommu_free
function.
Suggested-by: Robin Murphy
Signed-off-by: Vijayanand Jitta
---
drivers/iommu/iommu.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d410311..1d320ee 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -186,9 +186,14 @@ static struct dev_iommu *dev_iommu_get(struct device *dev)
static void dev_iommu_free(struct device *dev)
{
- iommu_fwspec_free(dev);
- kfree(dev->iommu);
+ struct dev_iommu *param = dev->iommu;
+
dev->iommu = NULL;
+ if (param->fwspec) {
+ fwnode_handle_put(param->fwspec->iommu_fwnode);
+ kfree(param->fwspec);
+ }
+ kfree(param);
}
static int __iommu_probe_device(struct device *dev, struct list_head
*group_list)
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] iommu: Fix potential use-after-free during probe
On 1/22/2022 12:50 AM, Robin Murphy wrote: > On 2022-01-21 07:16, Vijayanand Jitta wrote: >> >> >> On 1/18/2022 9:27 PM, Vijayanand Jitta wrote: >>> >>> >>> On 1/18/2022 7:19 PM, Robin Murphy wrote: >>>> On 2022-01-12 13:13, Vijayanand Jitta wrote: >>>>> Kasan has reported the following use after free on dev->iommu. >>>>> when a device probe fails and it is in process of freeing dev->iommu >>>>> in dev_iommu_free function, a deferred_probe_work_func runs in >>>>> parallel >>>>> and tries to access dev->iommu->fwspec in of_iommu_configure path thus >>>>> causing use after free. >>>>> >>>>> BUG: KASAN: use-after-free in of_iommu_configure+0xb4/0x4a4 >>>>> Read of size 8 at addr ff87a2f1acb8 by task kworker/u16:2/153 >>>>> >>>>> Workqueue: events_unbound deferred_probe_work_func >>>>> Call trace: >>>>> dump_backtrace+0x0/0x33c >>>>> show_stack+0x18/0x24 >>>>> dump_stack_lvl+0x16c/0x1e0 >>>>> print_address_description+0x84/0x39c >>>>> __kasan_report+0x184/0x308 >>>>> kasan_report+0x50/0x78 >>>>> __asan_load8+0xc0/0xc4 >>>>> of_iommu_configure+0xb4/0x4a4 >>>>> of_dma_configure_id+0x2fc/0x4d4 >>>>> platform_dma_configure+0x40/0x5c >>>>> really_probe+0x1b4/0xb74 >>>>> driver_probe_device+0x11c/0x228 >>>>> __device_attach_driver+0x14c/0x304 >>>>> bus_for_each_drv+0x124/0x1b0 >>>>> __device_attach+0x25c/0x334 >>>>> device_initial_probe+0x24/0x34 >>>>> bus_probe_device+0x78/0x134 >>>>> deferred_probe_work_func+0x130/0x1a8 >>>>> process_one_work+0x4c8/0x970 >>>>> worker_thread+0x5c8/0xaec >>>>> kthread+0x1f8/0x220 >>>>> ret_from_fork+0x10/0x18 >>>>> >>>>> Allocated by task 1: >>>>> kasan_kmalloc+0xd4/0x114 >>>>> __kasan_kmalloc+0x10/0x1c >>>>> kmem_cache_alloc_trace+0xe4/0x3d4 >>>>> __iommu_probe_device+0x90/0x394 >>>>> probe_iommu_group+0x70/0x9c >>>>> bus_for_each_dev+0x11c/0x19c >>>>> bus_iommu_probe+0xb8/0x7d4 >>>>> bus_set_iommu+0xcc/0x13c >>>>> arm_smmu_bus_init+0x44/0x130 [arm_smmu] >>>>> arm_smmu_device_probe+0xb88/0xc54 [arm_smmu] >>>>> platform_drv_probe+0xe4/0x13c >>>>> really_probe+0x2c8/0xb74 >>>>> driver_probe_device+0x11c/0x228 >>>>> device_driver_attach+0xf0/0x16c >>>>> __driver_attach+0x80/0x320 >>>>> bus_for_each_dev+0x11c/0x19c >>>>> driver_attach+0x38/0x48 >>>>> bus_add_driver+0x1dc/0x3a4 >>>>> driver_register+0x18c/0x244 >>>>> __platform_driver_register+0x88/0x9c >>>>> init_module+0x64/0xff4 [arm_smmu] >>>>> do_one_initcall+0x17c/0x2f0 >>>>> do_init_module+0xe8/0x378 >>>>> load_module+0x3f80/0x4a40 >>>>> __se_sys_finit_module+0x1a0/0x1e4 >>>>> __arm64_sys_finit_module+0x44/0x58 >>>>> el0_svc_common+0x100/0x264 >>>>> do_el0_svc+0x38/0xa4 >>>>> el0_svc+0x20/0x30 >>>>> el0_sync_handler+0x68/0xac >>>>> el0_sync+0x160/0x180 >>>>> >>>>> Freed by task 1: >>>>> kasan_set_track+0x4c/0x84 >>>>> kasan_set_free_info+0x28/0x4c >>>>> kasan_slab_free+0x120/0x15c >>>>> __kasan_slab_free+0x18/0x28 >>>>> slab_free_freelist_hook+0x204/0x2fc >>>>> kfree+0xfc/0x3a4 >>>>> __iommu_probe_device+0x284/0x394 >>>>> probe_iommu_group+0x70/0x9c >>>>> bus_for_each_dev+0x11c/0x19c >>>>> bus_iommu_probe+0xb8/0x7d4 >>>>> bus_set_iommu+0xcc/0x13c >>>>> arm_smmu_bus_init+0x44/0x130 [arm_smmu] >>>>> arm_smmu_device_probe+0xb88/0xc54 [arm_smmu] >>>>> platform_drv_probe+0xe4/0x13c >>>>> really_probe+0x2c8/0xb74 >>>>> driver_probe_device+0x11c/0x228 >>>>> device_driver_attach+0xf0/0x16c >>>>> __driver_attach+0x80/0x320 >>>>> bus_for_each_de
Re: [PATCH v3] iommu: Fix potential use-after-free during probe
On 1/18/2022 9:27 PM, Vijayanand Jitta wrote: > > > On 1/18/2022 7:19 PM, Robin Murphy wrote: >> On 2022-01-12 13:13, Vijayanand Jitta wrote: >>> Kasan has reported the following use after free on dev->iommu. >>> when a device probe fails and it is in process of freeing dev->iommu >>> in dev_iommu_free function, a deferred_probe_work_func runs in parallel >>> and tries to access dev->iommu->fwspec in of_iommu_configure path thus >>> causing use after free. >>> >>> BUG: KASAN: use-after-free in of_iommu_configure+0xb4/0x4a4 >>> Read of size 8 at addr ff87a2f1acb8 by task kworker/u16:2/153 >>> >>> Workqueue: events_unbound deferred_probe_work_func >>> Call trace: >>> dump_backtrace+0x0/0x33c >>> show_stack+0x18/0x24 >>> dump_stack_lvl+0x16c/0x1e0 >>> print_address_description+0x84/0x39c >>> __kasan_report+0x184/0x308 >>> kasan_report+0x50/0x78 >>> __asan_load8+0xc0/0xc4 >>> of_iommu_configure+0xb4/0x4a4 >>> of_dma_configure_id+0x2fc/0x4d4 >>> platform_dma_configure+0x40/0x5c >>> really_probe+0x1b4/0xb74 >>> driver_probe_device+0x11c/0x228 >>> __device_attach_driver+0x14c/0x304 >>> bus_for_each_drv+0x124/0x1b0 >>> __device_attach+0x25c/0x334 >>> device_initial_probe+0x24/0x34 >>> bus_probe_device+0x78/0x134 >>> deferred_probe_work_func+0x130/0x1a8 >>> process_one_work+0x4c8/0x970 >>> worker_thread+0x5c8/0xaec >>> kthread+0x1f8/0x220 >>> ret_from_fork+0x10/0x18 >>> >>> Allocated by task 1: >>> kasan_kmalloc+0xd4/0x114 >>> __kasan_kmalloc+0x10/0x1c >>> kmem_cache_alloc_trace+0xe4/0x3d4 >>> __iommu_probe_device+0x90/0x394 >>> probe_iommu_group+0x70/0x9c >>> bus_for_each_dev+0x11c/0x19c >>> bus_iommu_probe+0xb8/0x7d4 >>> bus_set_iommu+0xcc/0x13c >>> arm_smmu_bus_init+0x44/0x130 [arm_smmu] >>> arm_smmu_device_probe+0xb88/0xc54 [arm_smmu] >>> platform_drv_probe+0xe4/0x13c >>> really_probe+0x2c8/0xb74 >>> driver_probe_device+0x11c/0x228 >>> device_driver_attach+0xf0/0x16c >>> __driver_attach+0x80/0x320 >>> bus_for_each_dev+0x11c/0x19c >>> driver_attach+0x38/0x48 >>> bus_add_driver+0x1dc/0x3a4 >>> driver_register+0x18c/0x244 >>> __platform_driver_register+0x88/0x9c >>> init_module+0x64/0xff4 [arm_smmu] >>> do_one_initcall+0x17c/0x2f0 >>> do_init_module+0xe8/0x378 >>> load_module+0x3f80/0x4a40 >>> __se_sys_finit_module+0x1a0/0x1e4 >>> __arm64_sys_finit_module+0x44/0x58 >>> el0_svc_common+0x100/0x264 >>> do_el0_svc+0x38/0xa4 >>> el0_svc+0x20/0x30 >>> el0_sync_handler+0x68/0xac >>> el0_sync+0x160/0x180 >>> >>> Freed by task 1: >>> kasan_set_track+0x4c/0x84 >>> kasan_set_free_info+0x28/0x4c >>> kasan_slab_free+0x120/0x15c >>> __kasan_slab_free+0x18/0x28 >>> slab_free_freelist_hook+0x204/0x2fc >>> kfree+0xfc/0x3a4 >>> __iommu_probe_device+0x284/0x394 >>> probe_iommu_group+0x70/0x9c >>> bus_for_each_dev+0x11c/0x19c >>> bus_iommu_probe+0xb8/0x7d4 >>> bus_set_iommu+0xcc/0x13c >>> arm_smmu_bus_init+0x44/0x130 [arm_smmu] >>> arm_smmu_device_probe+0xb88/0xc54 [arm_smmu] >>> platform_drv_probe+0xe4/0x13c >>> really_probe+0x2c8/0xb74 >>> driver_probe_device+0x11c/0x228 >>> device_driver_attach+0xf0/0x16c >>> __driver_attach+0x80/0x320 >>> bus_for_each_dev+0x11c/0x19c >>> driver_attach+0x38/0x48 >>> bus_add_driver+0x1dc/0x3a4 >>> driver_register+0x18c/0x244 >>> __platform_driver_register+0x88/0x9c >>> init_module+0x64/0xff4 [arm_smmu] >>> do_one_initcall+0x17c/0x2f0 >>> do_init_module+0xe8/0x378 >>> load_module+0x3f80/0x4a40 >>> __se_sys_finit_module+0x1a0/0x1e4 >>> __arm64_sys_finit_module+0x44/0x58 >>> el0_svc_common+0x100/0x264 >>> do_el0_svc+0x38/0xa4 >>> el0_svc+0x20/0x30 >>> el0_sync_handler+0x68/0xac >>> el0_sync+0x160/0x180 >>> >>> Fix this by taking device_lock during probe_iommu_group. >>> >>> Signed-off-by: Vijayanand Jitta >>> --- >>> drivers/iommu/iommu.c | 12 >>> 1 file changed, 8 insertions(+), 4 deletions(-) >>>
Re: [PATCH v3] iommu: Fix potential use-after-free during probe
On 1/18/2022 7:19 PM, Robin Murphy wrote:
> On 2022-01-12 13:13, Vijayanand Jitta wrote:
>> Kasan has reported the following use after free on dev->iommu.
>> when a device probe fails and it is in process of freeing dev->iommu
>> in dev_iommu_free function, a deferred_probe_work_func runs in parallel
>> and tries to access dev->iommu->fwspec in of_iommu_configure path thus
>> causing use after free.
>>
>> BUG: KASAN: use-after-free in of_iommu_configure+0xb4/0x4a4
>> Read of size 8 at addr ff87a2f1acb8 by task kworker/u16:2/153
>>
>> Workqueue: events_unbound deferred_probe_work_func
>> Call trace:
>> dump_backtrace+0x0/0x33c
>> show_stack+0x18/0x24
>> dump_stack_lvl+0x16c/0x1e0
>> print_address_description+0x84/0x39c
>> __kasan_report+0x184/0x308
>> kasan_report+0x50/0x78
>> __asan_load8+0xc0/0xc4
>> of_iommu_configure+0xb4/0x4a4
>> of_dma_configure_id+0x2fc/0x4d4
>> platform_dma_configure+0x40/0x5c
>> really_probe+0x1b4/0xb74
>> driver_probe_device+0x11c/0x228
>> __device_attach_driver+0x14c/0x304
>> bus_for_each_drv+0x124/0x1b0
>> __device_attach+0x25c/0x334
>> device_initial_probe+0x24/0x34
>> bus_probe_device+0x78/0x134
>> deferred_probe_work_func+0x130/0x1a8
>> process_one_work+0x4c8/0x970
>> worker_thread+0x5c8/0xaec
>> kthread+0x1f8/0x220
>> ret_from_fork+0x10/0x18
>>
>> Allocated by task 1:
>> kasan_kmalloc+0xd4/0x114
>> __kasan_kmalloc+0x10/0x1c
>> kmem_cache_alloc_trace+0xe4/0x3d4
>> __iommu_probe_device+0x90/0x394
>> probe_iommu_group+0x70/0x9c
>> bus_for_each_dev+0x11c/0x19c
>> bus_iommu_probe+0xb8/0x7d4
>> bus_set_iommu+0xcc/0x13c
>> arm_smmu_bus_init+0x44/0x130 [arm_smmu]
>> arm_smmu_device_probe+0xb88/0xc54 [arm_smmu]
>> platform_drv_probe+0xe4/0x13c
>> really_probe+0x2c8/0xb74
>> driver_probe_device+0x11c/0x228
>> device_driver_attach+0xf0/0x16c
>> __driver_attach+0x80/0x320
>> bus_for_each_dev+0x11c/0x19c
>> driver_attach+0x38/0x48
>> bus_add_driver+0x1dc/0x3a4
>> driver_register+0x18c/0x244
>> __platform_driver_register+0x88/0x9c
>> init_module+0x64/0xff4 [arm_smmu]
>> do_one_initcall+0x17c/0x2f0
>> do_init_module+0xe8/0x378
>> load_module+0x3f80/0x4a40
>> __se_sys_finit_module+0x1a0/0x1e4
>> __arm64_sys_finit_module+0x44/0x58
>> el0_svc_common+0x100/0x264
>> do_el0_svc+0x38/0xa4
>> el0_svc+0x20/0x30
>> el0_sync_handler+0x68/0xac
>> el0_sync+0x160/0x180
>>
>> Freed by task 1:
>> kasan_set_track+0x4c/0x84
>> kasan_set_free_info+0x28/0x4c
>> kasan_slab_free+0x120/0x15c
>> __kasan_slab_free+0x18/0x28
>> slab_free_freelist_hook+0x204/0x2fc
>> kfree+0xfc/0x3a4
>> __iommu_probe_device+0x284/0x394
>> probe_iommu_group+0x70/0x9c
>> bus_for_each_dev+0x11c/0x19c
>> bus_iommu_probe+0xb8/0x7d4
>> bus_set_iommu+0xcc/0x13c
>> arm_smmu_bus_init+0x44/0x130 [arm_smmu]
>> arm_smmu_device_probe+0xb88/0xc54 [arm_smmu]
>> platform_drv_probe+0xe4/0x13c
>> really_probe+0x2c8/0xb74
>> driver_probe_device+0x11c/0x228
>> device_driver_attach+0xf0/0x16c
>> __driver_attach+0x80/0x320
>> bus_for_each_dev+0x11c/0x19c
>> driver_attach+0x38/0x48
>> bus_add_driver+0x1dc/0x3a4
>> driver_register+0x18c/0x244
>> __platform_driver_register+0x88/0x9c
>> init_module+0x64/0xff4 [arm_smmu]
>> do_one_initcall+0x17c/0x2f0
>> do_init_module+0xe8/0x378
>> load_module+0x3f80/0x4a40
>> __se_sys_finit_module+0x1a0/0x1e4
>> __arm64_sys_finit_module+0x44/0x58
>> el0_svc_common+0x100/0x264
>> do_el0_svc+0x38/0xa4
>> el0_svc+0x20/0x30
>> el0_sync_handler+0x68/0xac
>> el0_sync+0x160/0x180
>>
>> Fix this by taking device_lock during probe_iommu_group.
>>
>> Signed-off-by: Vijayanand Jitta
>> ---
>> drivers/iommu/iommu.c | 12
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index dd7863e..261792d 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1617,7 +1617,7 @@ static int probe_iommu_group(struct device *dev,
>> void *data)
>> {
>> struct list_head *group_list = data;
>> struct iommu_group *group;
>> - int ret;
>> + int ret = 0;
>> /* Device is probed already if in a group */
[PATCH v3] iommu: Fix potential use-after-free during probe
Kasan has reported the following use after free on dev->iommu.
when a device probe fails and it is in process of freeing dev->iommu
in dev_iommu_free function, a deferred_probe_work_func runs in parallel
and tries to access dev->iommu->fwspec in of_iommu_configure path thus
causing use after free.
BUG: KASAN: use-after-free in of_iommu_configure+0xb4/0x4a4
Read of size 8 at addr ff87a2f1acb8 by task kworker/u16:2/153
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace+0x0/0x33c
show_stack+0x18/0x24
dump_stack_lvl+0x16c/0x1e0
print_address_description+0x84/0x39c
__kasan_report+0x184/0x308
kasan_report+0x50/0x78
__asan_load8+0xc0/0xc4
of_iommu_configure+0xb4/0x4a4
of_dma_configure_id+0x2fc/0x4d4
platform_dma_configure+0x40/0x5c
really_probe+0x1b4/0xb74
driver_probe_device+0x11c/0x228
__device_attach_driver+0x14c/0x304
bus_for_each_drv+0x124/0x1b0
__device_attach+0x25c/0x334
device_initial_probe+0x24/0x34
bus_probe_device+0x78/0x134
deferred_probe_work_func+0x130/0x1a8
process_one_work+0x4c8/0x970
worker_thread+0x5c8/0xaec
kthread+0x1f8/0x220
ret_from_fork+0x10/0x18
Allocated by task 1:
kasan_kmalloc+0xd4/0x114
__kasan_kmalloc+0x10/0x1c
kmem_cache_alloc_trace+0xe4/0x3d4
__iommu_probe_device+0x90/0x394
probe_iommu_group+0x70/0x9c
bus_for_each_dev+0x11c/0x19c
bus_iommu_probe+0xb8/0x7d4
bus_set_iommu+0xcc/0x13c
arm_smmu_bus_init+0x44/0x130 [arm_smmu]
arm_smmu_device_probe+0xb88/0xc54 [arm_smmu]
platform_drv_probe+0xe4/0x13c
really_probe+0x2c8/0xb74
driver_probe_device+0x11c/0x228
device_driver_attach+0xf0/0x16c
__driver_attach+0x80/0x320
bus_for_each_dev+0x11c/0x19c
driver_attach+0x38/0x48
bus_add_driver+0x1dc/0x3a4
driver_register+0x18c/0x244
__platform_driver_register+0x88/0x9c
init_module+0x64/0xff4 [arm_smmu]
do_one_initcall+0x17c/0x2f0
do_init_module+0xe8/0x378
load_module+0x3f80/0x4a40
__se_sys_finit_module+0x1a0/0x1e4
__arm64_sys_finit_module+0x44/0x58
el0_svc_common+0x100/0x264
do_el0_svc+0x38/0xa4
el0_svc+0x20/0x30
el0_sync_handler+0x68/0xac
el0_sync+0x160/0x180
Freed by task 1:
kasan_set_track+0x4c/0x84
kasan_set_free_info+0x28/0x4c
kasan_slab_free+0x120/0x15c
__kasan_slab_free+0x18/0x28
slab_free_freelist_hook+0x204/0x2fc
kfree+0xfc/0x3a4
__iommu_probe_device+0x284/0x394
probe_iommu_group+0x70/0x9c
bus_for_each_dev+0x11c/0x19c
bus_iommu_probe+0xb8/0x7d4
bus_set_iommu+0xcc/0x13c
arm_smmu_bus_init+0x44/0x130 [arm_smmu]
arm_smmu_device_probe+0xb88/0xc54 [arm_smmu]
platform_drv_probe+0xe4/0x13c
really_probe+0x2c8/0xb74
driver_probe_device+0x11c/0x228
device_driver_attach+0xf0/0x16c
__driver_attach+0x80/0x320
bus_for_each_dev+0x11c/0x19c
driver_attach+0x38/0x48
bus_add_driver+0x1dc/0x3a4
driver_register+0x18c/0x244
__platform_driver_register+0x88/0x9c
init_module+0x64/0xff4 [arm_smmu]
do_one_initcall+0x17c/0x2f0
do_init_module+0xe8/0x378
load_module+0x3f80/0x4a40
__se_sys_finit_module+0x1a0/0x1e4
__arm64_sys_finit_module+0x44/0x58
el0_svc_common+0x100/0x264
do_el0_svc+0x38/0xa4
el0_svc+0x20/0x30
el0_sync_handler+0x68/0xac
el0_sync+0x160/0x180
Fix this by taking device_lock during probe_iommu_group.
Signed-off-by: Vijayanand Jitta
---
drivers/iommu/iommu.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index dd7863e..261792d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1617,7 +1617,7 @@ static int probe_iommu_group(struct device *dev, void
*data)
{
struct list_head *group_list = data;
struct iommu_group *group;
- int ret;
+ int ret = 0;
/* Device is probed already if in a group */
group = iommu_group_get(dev);
@@ -1626,9 +1626,13 @@ static int probe_iommu_group(struct device *dev, void
*data)
return 0;
}
- ret = __iommu_probe_device(dev, group_list);
- if (ret == -ENODEV)
- ret = 0;
+ ret = device_trylock(dev);
+ if (ret) {
+ ret = __iommu_probe_device(dev, group_list);
+ if (ret == -ENODEV)
+ ret = 0;
+ device_unlock(dev);
+ }
return ret;
}
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 2/2] iommu/iova: Free global iova rcache on iova alloc failure
On 9/30/2020 1:14 PM, vji...@codeaurora.org wrote:
> From: Vijayanand Jitta
>
> When ever an iova alloc request fails we free the iova
> ranges present in the percpu iova rcaches and then retry
> but the global iova rcache is not freed as a result we could
> still see iova alloc failure even after retry as global
> rcache is holding the iova's which can cause fragmentation.
> So, free the global iova rcache as well and then go for the
> retry.
>
> Signed-off-by: Vijayanand Jitta
> ---
> drivers/iommu/iova.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index c3a1a8e..faf9b13 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -25,6 +25,7 @@ static void init_iova_rcaches(struct iova_domain *iovad);
> static void free_iova_rcaches(struct iova_domain *iovad);
> static void fq_destroy_all_entries(struct iova_domain *iovad);
> static void fq_flush_timeout(struct timer_list *t);
> +static void free_global_cached_iovas(struct iova_domain *iovad);
>
> void
> init_iova_domain(struct iova_domain *iovad, unsigned long granule,
> @@ -442,6 +443,7 @@ alloc_iova_fast(struct iova_domain *iovad, unsigned long
> size,
> flush_rcache = false;
> for_each_online_cpu(cpu)
> free_cpu_cached_iovas(cpu, iovad);
> + free_global_cached_iovas(iovad);
> goto retry;
> }
>
> @@ -1057,5 +1059,26 @@ void free_cpu_cached_iovas(unsigned int cpu, struct
> iova_domain *iovad)
> }
> }
>
> +/*
> + * free all the IOVA ranges of global cache
> + */
> +static void free_global_cached_iovas(struct iova_domain *iovad)
> +{
> + struct iova_rcache *rcache;
> + unsigned long flags;
> + int i, j;
> +
> + for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
> + rcache = >rcaches[i];
> + spin_lock_irqsave(>lock, flags);
> + for (j = 0; j < rcache->depot_size; ++j) {
> + iova_magazine_free_pfns(rcache->depot[j], iovad);
> + iova_magazine_free(rcache->depot[j]);
> + rcache->depot[j] = NULL;
> + }
> + rcache->depot_size = 0;
> + spin_unlock_irqrestore(>lock, flags);
> + }
> +}
> MODULE_AUTHOR("Anil S Keshavamurthy ");
> MODULE_LICENSE("GPL");
>
Gentle ping.
Thanks,
Vijay
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 1/2] iommu/iova: Retry from last rb tree node if iova search fails
On 9/30/2020 1:14 PM, vji...@codeaurora.org wrote:
> From: Vijayanand Jitta
>
> When ever a new iova alloc request comes iova is always searched
> from the cached node and the nodes which are previous to cached
> node. So, even if there is free iova space available in the nodes
> which are next to the cached node iova allocation can still fail
> because of this approach.
>
> Consider the following sequence of iova alloc and frees on
> 1GB of iova space
>
> 1) alloc - 500MB
> 2) alloc - 12MB
> 3) alloc - 499MB
> 4) free - 12MB which was allocated in step 2
> 5) alloc - 13MB
>
> After the above sequence we will have 12MB of free iova space and
> cached node will be pointing to the iova pfn of last alloc of 13MB
> which will be the lowest iova pfn of that iova space. Now if we get an
> alloc request of 2MB we just search from cached node and then look
> for lower iova pfn's for free iova and as they aren't any, iova alloc
> fails though there is 12MB of free iova space.
>
> To avoid such iova search failures do a retry from the last rb tree node
> when iova search fails, this will search the entire tree and get an iova
> if its available.
>
> Signed-off-by: Vijayanand Jitta
> Reviewed-by: Robin Murphy
> ---
> drivers/iommu/iova.c | 23 +--
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 30d969a..c3a1a8e 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -184,8 +184,9 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
> struct rb_node *curr, *prev;
> struct iova *curr_iova;
> unsigned long flags;
> - unsigned long new_pfn;
> + unsigned long new_pfn, retry_pfn;
> unsigned long align_mask = ~0UL;
> + unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
>
> if (size_aligned)
> align_mask <<= fls_long(size - 1);
> @@ -198,15 +199,25 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
>
> curr = __get_cached_rbnode(iovad, limit_pfn);
> curr_iova = rb_entry(curr, struct iova, node);
> + retry_pfn = curr_iova->pfn_hi + 1;
> +
> +retry:
> do {
> - limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
> - new_pfn = (limit_pfn - size) & align_mask;
> + high_pfn = min(high_pfn, curr_iova->pfn_lo);
> + new_pfn = (high_pfn - size) & align_mask;
> prev = curr;
> curr = rb_prev(curr);
> curr_iova = rb_entry(curr, struct iova, node);
> - } while (curr && new_pfn <= curr_iova->pfn_hi);
> -
> - if (limit_pfn < size || new_pfn < iovad->start_pfn) {
> + } while (curr && new_pfn <= curr_iova->pfn_hi && new_pfn >= low_pfn);
> +
> + if (high_pfn < size || new_pfn < low_pfn) {
> + if (low_pfn == iovad->start_pfn && retry_pfn < limit_pfn) {
> + high_pfn = limit_pfn;
> + low_pfn = retry_pfn;
> + curr = >anchor.node;
> + curr_iova = rb_entry(curr, struct iova, node);
> + goto retry;
> + }
> iovad->max32_alloc_size = size;
> goto iova32_full;
> }
>
Gentle ping.
Thanks,
Vijay
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/2] iommu/iova: Free global iova rcache on iova alloc failure
On 9/28/2020 6:11 PM, Vijayanand Jitta wrote:
>
>
> On 9/18/2020 8:11 PM, Robin Murphy wrote:
>> On 2020-08-20 13:49, vji...@codeaurora.org wrote:
>>> From: Vijayanand Jitta
>>>
>>> When ever an iova alloc request fails we free the iova
>>> ranges present in the percpu iova rcaches and then retry
>>> but the global iova rcache is not freed as a result we could
>>> still see iova alloc failure even after retry as global
>>> rcache is holding the iova's which can cause fragmentation.
>>> So, free the global iova rcache as well and then go for the
>>> retry.
>>>
>>> Signed-off-by: Vijayanand Jitta
>>> ---
>>> drivers/iommu/iova.c | 23 +++
>>> include/linux/iova.h | 6 ++
>>> 2 files changed, 29 insertions(+)
>>>
>>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>>> index 4e77116..5836c87 100644
>>> --- a/drivers/iommu/iova.c
>>> +++ b/drivers/iommu/iova.c
>>> @@ -442,6 +442,7 @@ struct iova *find_iova(struct iova_domain *iovad,
>>> unsigned long pfn)
>>> flush_rcache = false;
>>> for_each_online_cpu(cpu)
>>> free_cpu_cached_iovas(cpu, iovad);
>>> + free_global_cached_iovas(iovad);
>>> goto retry;
>>> }
>>> @@ -1055,5 +1056,27 @@ void free_cpu_cached_iovas(unsigned int cpu,
>>> struct iova_domain *iovad)
>>> }
>>> }
>>> +/*
>>> + * free all the IOVA ranges of global cache
>>> + */
>>> +void free_global_cached_iovas(struct iova_domain *iovad)
>>
>> As John pointed out last time, this should be static and the header
>> changes dropped.
>>
>> (TBH we should probably register our own hotplug notifier instance for a
>> flush queue, so that external code has no need to poke at the per-CPU
>> caches either)
>>
>> Robin.
>>
>
> Right, I have made it static and dropped header changes in v3.
> can you please review that.
>
> Thanks,
> Vijay
Please review v4 instead of v3, I have updated other patch as well in v4.
Thanks,
Vijay
>>> +{
>>> + struct iova_rcache *rcache;
>>> + unsigned long flags;
>>> + int i, j;
>>> +
>>> + for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
>>> + rcache = >rcaches[i];
>>> + spin_lock_irqsave(>lock, flags);
>>> + for (j = 0; j < rcache->depot_size; ++j) {
>>> + iova_magazine_free_pfns(rcache->depot[j], iovad);
>>> + iova_magazine_free(rcache->depot[j]);
>>> + rcache->depot[j] = NULL;
>>> + }
>>> + rcache->depot_size = 0;
>>> + spin_unlock_irqrestore(>lock, flags);
>>> + }
>>> +}
>>> +
>>> MODULE_AUTHOR("Anil S Keshavamurthy ");
>>> MODULE_LICENSE("GPL");
>>> diff --git a/include/linux/iova.h b/include/linux/iova.h
>>> index a0637ab..a905726 100644
>>> --- a/include/linux/iova.h
>>> +++ b/include/linux/iova.h
>>> @@ -163,6 +163,7 @@ int init_iova_flush_queue(struct iova_domain *iovad,
>>> struct iova *split_and_remove_iova(struct iova_domain *iovad,
>>> struct iova *iova, unsigned long pfn_lo, unsigned long pfn_hi);
>>> void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain
>>> *iovad);
>>> +void free_global_cached_iovas(struct iova_domain *iovad);
>>> #else
>>> static inline int iova_cache_get(void)
>>> {
>>> @@ -270,6 +271,11 @@ static inline void free_cpu_cached_iovas(unsigned
>>> int cpu,
>>> struct iova_domain *iovad)
>>> {
>>> }
>>> +
>>> +static inline void free_global_cached_iovas(struct iova_domain *iovad)
>>> +{
>>> +}
>>> +
>>> #endif
>>> #endif
>>>
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] iommu/iova: Retry from last rb tree node if iova search fails
On 9/18/2020 7:48 PM, Robin Murphy wrote:
> On 2020-08-20 13:49, vji...@codeaurora.org wrote:
>> From: Vijayanand Jitta
>>
>> When ever a new iova alloc request comes iova is always searched
>> from the cached node and the nodes which are previous to cached
>> node. So, even if there is free iova space available in the nodes
>> which are next to the cached node iova allocation can still fail
>> because of this approach.
>>
>> Consider the following sequence of iova alloc and frees on
>> 1GB of iova space
>>
>> 1) alloc - 500MB
>> 2) alloc - 12MB
>> 3) alloc - 499MB
>> 4) free - 12MB which was allocated in step 2
>> 5) alloc - 13MB
>>
>> After the above sequence we will have 12MB of free iova space and
>> cached node will be pointing to the iova pfn of last alloc of 13MB
>> which will be the lowest iova pfn of that iova space. Now if we get an
>> alloc request of 2MB we just search from cached node and then look
>> for lower iova pfn's for free iova and as they aren't any, iova alloc
>> fails though there is 12MB of free iova space.
>>
>> To avoid such iova search failures do a retry from the last rb tree node
>> when iova search fails, this will search the entire tree and get an iova
>> if its available.
>>
>> Signed-off-by: Vijayanand Jitta
>> ---
>> drivers/iommu/iova.c | 23 +--
>> 1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>> index 49fc01f..4e77116 100644
>> --- a/drivers/iommu/iova.c
>> +++ b/drivers/iommu/iova.c
>> @@ -184,8 +184,9 @@ static int __alloc_and_insert_iova_range(struct
>> iova_domain *iovad,
>> struct rb_node *curr, *prev;
>> struct iova *curr_iova;
>> unsigned long flags;
>> - unsigned long new_pfn;
>> + unsigned long new_pfn, low_pfn_new;
>> unsigned long align_mask = ~0UL;
>> + unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
>> if (size_aligned)
>> align_mask <<= fls_long(size - 1);
>> @@ -198,15 +199,25 @@ static int __alloc_and_insert_iova_range(struct
>> iova_domain *iovad,
>> curr = __get_cached_rbnode(iovad, limit_pfn);
>> curr_iova = rb_entry(curr, struct iova, node);
>> + low_pfn_new = curr_iova->pfn_hi + 1;
>
> Could we call "low_pfn_new" something like "retry_pfn" instead? This
> code already has unavoidable readability struggles with so many
> different "pfn"s in play, so having two different meanings of "new"
> really doesn't help.
>
> Other than that, I think this looks OK (IIRC it's basically what I
> originally suggested), so with the naming tweaked,
>
> Reviewed-by: Robin Murphy
>
Thanks for review, I have renamed it to retry_pfn in v4.
Thanks,
Vijay
>> +
>> +retry:
>> do {
>> - limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
>> - new_pfn = (limit_pfn - size) & align_mask;
>> + high_pfn = min(high_pfn, curr_iova->pfn_lo);
>> + new_pfn = (high_pfn - size) & align_mask;
>> prev = curr;
>> curr = rb_prev(curr);
>> curr_iova = rb_entry(curr, struct iova, node);
>> - } while (curr && new_pfn <= curr_iova->pfn_hi);
>> -
>> - if (limit_pfn < size || new_pfn < iovad->start_pfn) {
>> + } while (curr && new_pfn <= curr_iova->pfn_hi && new_pfn >=
>> low_pfn);
>> +
>> + if (high_pfn < size || new_pfn < low_pfn) {
>> + if (low_pfn == iovad->start_pfn && low_pfn_new < limit_pfn) {
>> + high_pfn = limit_pfn;
>> + low_pfn = low_pfn_new;
>> + curr = >anchor.node;
>> + curr_iova = rb_entry(curr, struct iova, node);
>> + goto retry;
>> + }
>> iovad->max32_alloc_size = size;
>> goto iova32_full;
>> }
>>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/2] iommu/iova: Free global iova rcache on iova alloc failure
On 9/18/2020 8:11 PM, Robin Murphy wrote:
> On 2020-08-20 13:49, vji...@codeaurora.org wrote:
>> From: Vijayanand Jitta
>>
>> When ever an iova alloc request fails we free the iova
>> ranges present in the percpu iova rcaches and then retry
>> but the global iova rcache is not freed as a result we could
>> still see iova alloc failure even after retry as global
>> rcache is holding the iova's which can cause fragmentation.
>> So, free the global iova rcache as well and then go for the
>> retry.
>>
>> Signed-off-by: Vijayanand Jitta
>> ---
>> drivers/iommu/iova.c | 23 +++
>> include/linux/iova.h | 6 ++
>> 2 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>> index 4e77116..5836c87 100644
>> --- a/drivers/iommu/iova.c
>> +++ b/drivers/iommu/iova.c
>> @@ -442,6 +442,7 @@ struct iova *find_iova(struct iova_domain *iovad,
>> unsigned long pfn)
>> flush_rcache = false;
>> for_each_online_cpu(cpu)
>> free_cpu_cached_iovas(cpu, iovad);
>> + free_global_cached_iovas(iovad);
>> goto retry;
>> }
>> @@ -1055,5 +1056,27 @@ void free_cpu_cached_iovas(unsigned int cpu,
>> struct iova_domain *iovad)
>> }
>> }
>> +/*
>> + * free all the IOVA ranges of global cache
>> + */
>> +void free_global_cached_iovas(struct iova_domain *iovad)
>
> As John pointed out last time, this should be static and the header
> changes dropped.
>
> (TBH we should probably register our own hotplug notifier instance for a
> flush queue, so that external code has no need to poke at the per-CPU
> caches either)
>
> Robin.
>
Right, I have made it static and dropped header changes in v3.
can you please review that.
Thanks,
Vijay
>> +{
>> + struct iova_rcache *rcache;
>> + unsigned long flags;
>> + int i, j;
>> +
>> + for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
>> + rcache = >rcaches[i];
>> + spin_lock_irqsave(>lock, flags);
>> + for (j = 0; j < rcache->depot_size; ++j) {
>> + iova_magazine_free_pfns(rcache->depot[j], iovad);
>> + iova_magazine_free(rcache->depot[j]);
>> + rcache->depot[j] = NULL;
>> + }
>> + rcache->depot_size = 0;
>> + spin_unlock_irqrestore(>lock, flags);
>> + }
>> +}
>> +
>> MODULE_AUTHOR("Anil S Keshavamurthy ");
>> MODULE_LICENSE("GPL");
>> diff --git a/include/linux/iova.h b/include/linux/iova.h
>> index a0637ab..a905726 100644
>> --- a/include/linux/iova.h
>> +++ b/include/linux/iova.h
>> @@ -163,6 +163,7 @@ int init_iova_flush_queue(struct iova_domain *iovad,
>> struct iova *split_and_remove_iova(struct iova_domain *iovad,
>> struct iova *iova, unsigned long pfn_lo, unsigned long pfn_hi);
>> void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain
>> *iovad);
>> +void free_global_cached_iovas(struct iova_domain *iovad);
>> #else
>> static inline int iova_cache_get(void)
>> {
>> @@ -270,6 +271,11 @@ static inline void free_cpu_cached_iovas(unsigned
>> int cpu,
>> struct iova_domain *iovad)
>> {
>> }
>> +
>> +static inline void free_global_cached_iovas(struct iova_domain *iovad)
>> +{
>> +}
>> +
>> #endif
>> #endif
>>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] iommu/iova: Retry from last rb tree node if iova search fails
On 8/28/2020 1:01 PM, Vijayanand Jitta wrote:
>
>
> On 8/20/2020 6:19 PM, vji...@codeaurora.org wrote:
>> From: Vijayanand Jitta
>>
>> When ever a new iova alloc request comes iova is always searched
>> from the cached node and the nodes which are previous to cached
>> node. So, even if there is free iova space available in the nodes
>> which are next to the cached node iova allocation can still fail
>> because of this approach.
>>
>> Consider the following sequence of iova alloc and frees on
>> 1GB of iova space
>>
>> 1) alloc - 500MB
>> 2) alloc - 12MB
>> 3) alloc - 499MB
>> 4) free - 12MB which was allocated in step 2
>> 5) alloc - 13MB
>>
>> After the above sequence we will have 12MB of free iova space and
>> cached node will be pointing to the iova pfn of last alloc of 13MB
>> which will be the lowest iova pfn of that iova space. Now if we get an
>> alloc request of 2MB we just search from cached node and then look
>> for lower iova pfn's for free iova and as they aren't any, iova alloc
>> fails though there is 12MB of free iova space.
>>
>> To avoid such iova search failures do a retry from the last rb tree node
>> when iova search fails, this will search the entire tree and get an iova
>> if its available.
>>
>> Signed-off-by: Vijayanand Jitta
>> ---
>> drivers/iommu/iova.c | 23 +--
>> 1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>> index 49fc01f..4e77116 100644
>> --- a/drivers/iommu/iova.c
>> +++ b/drivers/iommu/iova.c
>> @@ -184,8 +184,9 @@ static int __alloc_and_insert_iova_range(struct
>> iova_domain *iovad,
>> struct rb_node *curr, *prev;
>> struct iova *curr_iova;
>> unsigned long flags;
>> -unsigned long new_pfn;
>> +unsigned long new_pfn, low_pfn_new;
>> unsigned long align_mask = ~0UL;
>> +unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
>>
>> if (size_aligned)
>> align_mask <<= fls_long(size - 1);
>> @@ -198,15 +199,25 @@ static int __alloc_and_insert_iova_range(struct
>> iova_domain *iovad,
>>
>> curr = __get_cached_rbnode(iovad, limit_pfn);
>> curr_iova = rb_entry(curr, struct iova, node);
>> +low_pfn_new = curr_iova->pfn_hi + 1;
>> +
>> +retry:
>> do {
>> -limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
>> -new_pfn = (limit_pfn - size) & align_mask;
>> +high_pfn = min(high_pfn, curr_iova->pfn_lo);
>> +new_pfn = (high_pfn - size) & align_mask;
>> prev = curr;
>> curr = rb_prev(curr);
>> curr_iova = rb_entry(curr, struct iova, node);
>> -} while (curr && new_pfn <= curr_iova->pfn_hi);
>> -
>> -if (limit_pfn < size || new_pfn < iovad->start_pfn) {
>> +} while (curr && new_pfn <= curr_iova->pfn_hi && new_pfn >= low_pfn);
>> +
>> +if (high_pfn < size || new_pfn < low_pfn) {
>> +if (low_pfn == iovad->start_pfn && low_pfn_new < limit_pfn) {
>> +high_pfn = limit_pfn;
>> +low_pfn = low_pfn_new;
>> +curr = >anchor.node;
>> +curr_iova = rb_entry(curr, struct iova, node);
>> +goto retry;
>> +}
>> iovad->max32_alloc_size = size;
>> goto iova32_full;
>> }
>>
>
> ping ?
>
> Thanks,
> Vijay
>
ping ?
Thanks,
Vijay
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] iommu/iova: Retry from last rb tree node if iova search fails
On 8/20/2020 6:19 PM, vji...@codeaurora.org wrote:
> From: Vijayanand Jitta
>
> When ever a new iova alloc request comes iova is always searched
> from the cached node and the nodes which are previous to cached
> node. So, even if there is free iova space available in the nodes
> which are next to the cached node iova allocation can still fail
> because of this approach.
>
> Consider the following sequence of iova alloc and frees on
> 1GB of iova space
>
> 1) alloc - 500MB
> 2) alloc - 12MB
> 3) alloc - 499MB
> 4) free - 12MB which was allocated in step 2
> 5) alloc - 13MB
>
> After the above sequence we will have 12MB of free iova space and
> cached node will be pointing to the iova pfn of last alloc of 13MB
> which will be the lowest iova pfn of that iova space. Now if we get an
> alloc request of 2MB we just search from cached node and then look
> for lower iova pfn's for free iova and as they aren't any, iova alloc
> fails though there is 12MB of free iova space.
>
> To avoid such iova search failures do a retry from the last rb tree node
> when iova search fails, this will search the entire tree and get an iova
> if its available.
>
> Signed-off-by: Vijayanand Jitta
> ---
> drivers/iommu/iova.c | 23 +--
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 49fc01f..4e77116 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -184,8 +184,9 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
> struct rb_node *curr, *prev;
> struct iova *curr_iova;
> unsigned long flags;
> - unsigned long new_pfn;
> + unsigned long new_pfn, low_pfn_new;
> unsigned long align_mask = ~0UL;
> + unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
>
> if (size_aligned)
> align_mask <<= fls_long(size - 1);
> @@ -198,15 +199,25 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
>
> curr = __get_cached_rbnode(iovad, limit_pfn);
> curr_iova = rb_entry(curr, struct iova, node);
> + low_pfn_new = curr_iova->pfn_hi + 1;
> +
> +retry:
> do {
> - limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
> - new_pfn = (limit_pfn - size) & align_mask;
> + high_pfn = min(high_pfn, curr_iova->pfn_lo);
> + new_pfn = (high_pfn - size) & align_mask;
> prev = curr;
> curr = rb_prev(curr);
> curr_iova = rb_entry(curr, struct iova, node);
> - } while (curr && new_pfn <= curr_iova->pfn_hi);
> -
> - if (limit_pfn < size || new_pfn < iovad->start_pfn) {
> + } while (curr && new_pfn <= curr_iova->pfn_hi && new_pfn >= low_pfn);
> +
> + if (high_pfn < size || new_pfn < low_pfn) {
> + if (low_pfn == iovad->start_pfn && low_pfn_new < limit_pfn) {
> + high_pfn = limit_pfn;
> + low_pfn = low_pfn_new;
> + curr = >anchor.node;
> + curr_iova = rb_entry(curr, struct iova, node);
> + goto retry;
> + }
> iovad->max32_alloc_size = size;
> goto iova32_full;
> }
>
ping ?
Thanks,
Vijay
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/iova: Free global iova rcache on iova alloc failure
On 8/12/2020 8:46 PM, Joerg Roedel wrote: > On Mon, Aug 03, 2020 at 03:30:48PM +0530, Vijayanand Jitta wrote: >> ping? > > Please repost when v5.9-rc1 is released and add > > Robin Murphy > > on your Cc list. > > Thanks, > > Joerg > Sure, will do. Thanks, Vijay -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/iova: Free global iova rcache on iova alloc failure
On 7/3/2020 7:47 PM, vji...@codeaurora.org wrote:
> From: Vijayanand Jitta
>
> When ever an iova alloc request fails we free the iova
> ranges present in the percpu iova rcaches and then retry
> but the global iova rcache is not freed as a result we could
> still see iova alloc failure even after retry as global
> rcache is holding the iova's which can cause fragmentation.
> So, free the global iova rcache as well and then go for the
> retry.
>
> Change-Id: Ib8236dc88ba5516b73d4fbf6bf8e68bbf09bbad2
> Signed-off-by: Vijayanand Jitta
> ---
> drivers/iommu/iova.c | 23 +++
> include/linux/iova.h | 6 ++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 4e77116..5836c87 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -442,6 +442,7 @@ struct iova *find_iova(struct iova_domain *iovad,
> unsigned long pfn)
> flush_rcache = false;
> for_each_online_cpu(cpu)
> free_cpu_cached_iovas(cpu, iovad);
> + free_global_cached_iovas(iovad);
> goto retry;
> }
>
> @@ -1055,5 +1056,27 @@ void free_cpu_cached_iovas(unsigned int cpu, struct
> iova_domain *iovad)
> }
> }
>
> +/*
> + * free all the IOVA ranges of global cache
> + */
> +void free_global_cached_iovas(struct iova_domain *iovad)
> +{
> + struct iova_rcache *rcache;
> + unsigned long flags;
> + int i, j;
> +
> + for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
> + rcache = >rcaches[i];
> + spin_lock_irqsave(>lock, flags);
> + for (j = 0; j < rcache->depot_size; ++j) {
> + iova_magazine_free_pfns(rcache->depot[j], iovad);
> + iova_magazine_free(rcache->depot[j]);
> + rcache->depot[j] = NULL;
> + }
> + rcache->depot_size = 0;
> + spin_unlock_irqrestore(>lock, flags);
> + }
> +}
> +
> MODULE_AUTHOR("Anil S Keshavamurthy ");
> MODULE_LICENSE("GPL");
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index a0637ab..a905726 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h
> @@ -163,6 +163,7 @@ int init_iova_flush_queue(struct iova_domain *iovad,
> struct iova *split_and_remove_iova(struct iova_domain *iovad,
> struct iova *iova, unsigned long pfn_lo, unsigned long pfn_hi);
> void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain *iovad);
> +void free_global_cached_iovas(struct iova_domain *iovad);
> #else
> static inline int iova_cache_get(void)
> {
> @@ -270,6 +271,11 @@ static inline void free_cpu_cached_iovas(unsigned int
> cpu,
>struct iova_domain *iovad)
> {
> }
> +
> +static inline void free_global_cached_iovas(struct iova_domain *iovad)
> +{
> +}
> +
> #endif
>
> #endif
>
ping?
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu/iova: Retry from last rb tree node if iova search fails
On 7/3/2020 7:47 PM, vji...@codeaurora.org wrote:
> From: Vijayanand Jitta
>
> When ever a new iova alloc request comes iova is always searched
> from the cached node and the nodes which are previous to cached
> node. So, even if there is free iova space available in the nodes
> which are next to the cached node iova allocation can still fail
> because of this approach.
>
> Consider the following sequence of iova alloc and frees on
> 1GB of iova space
>
> 1) alloc - 500MB
> 2) alloc - 12MB
> 3) alloc - 499MB
> 4) free - 12MB which was allocated in step 2
> 5) alloc - 13MB
>
> After the above sequence we will have 12MB of free iova space and
> cached node will be pointing to the iova pfn of last alloc of 13MB
> which will be the lowest iova pfn of that iova space. Now if we get an
> alloc request of 2MB we just search from cached node and then look
> for lower iova pfn's for free iova and as they aren't any, iova alloc
> fails though there is 12MB of free iova space.
>
> To avoid such iova search failures do a retry from the last rb tree node
> when iova search fails, this will search the entire tree and get an iova
> if its available.
>
> Signed-off-by: Vijayanand Jitta
> ---
> drivers/iommu/iova.c | 23 +--
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 49fc01f..4e77116 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -184,8 +184,9 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
> struct rb_node *curr, *prev;
> struct iova *curr_iova;
> unsigned long flags;
> - unsigned long new_pfn;
> + unsigned long new_pfn, low_pfn_new;
> unsigned long align_mask = ~0UL;
> + unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
>
> if (size_aligned)
> align_mask <<= fls_long(size - 1);
> @@ -198,15 +199,25 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
>
> curr = __get_cached_rbnode(iovad, limit_pfn);
> curr_iova = rb_entry(curr, struct iova, node);
> + low_pfn_new = curr_iova->pfn_hi + 1;
> +
> +retry:
> do {
> - limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
> - new_pfn = (limit_pfn - size) & align_mask;
> + high_pfn = min(high_pfn, curr_iova->pfn_lo);
> + new_pfn = (high_pfn - size) & align_mask;
> prev = curr;
> curr = rb_prev(curr);
> curr_iova = rb_entry(curr, struct iova, node);
> - } while (curr && new_pfn <= curr_iova->pfn_hi);
> -
> - if (limit_pfn < size || new_pfn < iovad->start_pfn) {
> + } while (curr && new_pfn <= curr_iova->pfn_hi && new_pfn >= low_pfn);
> +
> + if (high_pfn < size || new_pfn < low_pfn) {
> + if (low_pfn == iovad->start_pfn && low_pfn_new < limit_pfn) {
> + high_pfn = limit_pfn;
> + low_pfn = low_pfn_new;
> + curr = >anchor.node;
> + curr_iova = rb_entry(curr, struct iova, node);
> + goto retry;
> + }
> iovad->max32_alloc_size = size;
> goto iova32_full;
> }
>
ping?
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/iova: Retry from last rb tree node if iova search fails
On 5/11/2020 4:34 PM, vji...@codeaurora.org wrote:
> From: Vijayanand Jitta
>
> When ever a new iova alloc request comes iova is always searched
> from the cached node and the nodes which are previous to cached
> node. So, even if there is free iova space available in the nodes
> which are next to the cached node iova allocation can still fail
> because of this approach.
>
> Consider the following sequence of iova alloc and frees on
> 1GB of iova space
>
> 1) alloc - 500MB
> 2) alloc - 12MB
> 3) alloc - 499MB
> 4) free - 12MB which was allocated in step 2
> 5) alloc - 13MB
>
> After the above sequence we will have 12MB of free iova space and
> cached node will be pointing to the iova pfn of last alloc of 13MB
> which will be the lowest iova pfn of that iova space. Now if we get an
> alloc request of 2MB we just search from cached node and then look
> for lower iova pfn's for free iova and as they aren't any, iova alloc
> fails though there is 12MB of free iova space.
>
> To avoid such iova search failures do a retry from the last rb tree node
> when iova search fails, this will search the entire tree and get an iova
> if its available
>
> Signed-off-by: Vijayanand Jitta
> ---
> drivers/iommu/iova.c | 19 +++
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 0e6a953..7d82afc 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -184,8 +184,9 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
> struct rb_node *curr, *prev;
> struct iova *curr_iova;
> unsigned long flags;
> - unsigned long new_pfn;
> + unsigned long new_pfn, alloc_lo_new;
> unsigned long align_mask = ~0UL;
> + unsigned long alloc_hi = limit_pfn, alloc_lo = iovad->start_pfn;
>
> if (size_aligned)
> align_mask <<= fls_long(size - 1);
> @@ -198,15 +199,25 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
>
> curr = __get_cached_rbnode(iovad, limit_pfn);
> curr_iova = rb_entry(curr, struct iova, node);
> + alloc_lo_new = curr_iova->pfn_hi;
> +
> +retry:
> do {
> - limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
> - new_pfn = (limit_pfn - size) & align_mask;
> + alloc_hi = min(alloc_hi, curr_iova->pfn_lo);
> + new_pfn = (alloc_hi - size) & align_mask;
> prev = curr;
> curr = rb_prev(curr);
> curr_iova = rb_entry(curr, struct iova, node);
> } while (curr && new_pfn <= curr_iova->pfn_hi);
>
> - if (limit_pfn < size || new_pfn < iovad->start_pfn) {
> + if (alloc_hi < size || new_pfn < alloc_lo) {
> + if (alloc_lo == iovad->start_pfn && alloc_lo_new < limit_pfn) {
> + alloc_hi = limit_pfn;
> + alloc_lo = alloc_lo_new;
> + curr = >anchor.node;
> + curr_iova = rb_entry(curr, struct iova, node);
> + goto retry;
> + }
> iovad->max32_alloc_size = size;
> goto iova32_full;
> }
>
ping?
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/iova: Retry from last rb tree node if iova search fails
On 5/9/2020 12:25 AM, Vijayanand Jitta wrote:
>
>
> On 5/7/2020 6:54 PM, Robin Murphy wrote:
>> On 2020-05-06 9:01 pm, vji...@codeaurora.org wrote:
>>> From: Vijayanand Jitta
>>>
>>> When ever a new iova alloc request comes iova is always searched
>>> from the cached node and the nodes which are previous to cached
>>> node. So, even if there is free iova space available in the nodes
>>> which are next to the cached node iova allocation can still fail
>>> because of this approach.
>>>
>>> Consider the following sequence of iova alloc and frees on
>>> 1GB of iova space
>>>
>>> 1) alloc - 500MB
>>> 2) alloc - 12MB
>>> 3) alloc - 499MB
>>> 4) free - 12MB which was allocated in step 2
>>> 5) alloc - 13MB
>>>
>>> After the above sequence we will have 12MB of free iova space and
>>> cached node will be pointing to the iova pfn of last alloc of 13MB
>>> which will be the lowest iova pfn of that iova space. Now if we get an
>>> alloc request of 2MB we just search from cached node and then look
>>> for lower iova pfn's for free iova and as they aren't any, iova alloc
>>> fails though there is 12MB of free iova space.
>>
>> Yup, this could definitely do with improving. Unfortunately I think this
>> particular implementation is slightly flawed...
>>
>>> To avoid such iova search failures do a retry from the last rb tree node
>>> when iova search fails, this will search the entire tree and get an iova
>>> if its available
>>>
>>> Signed-off-by: Vijayanand Jitta
>>> ---
>>> drivers/iommu/iova.c | 11 +++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>>> index 0e6a953..2985222 100644
>>> --- a/drivers/iommu/iova.c
>>> +++ b/drivers/iommu/iova.c
>>> @@ -186,6 +186,7 @@ static int __alloc_and_insert_iova_range(struct
>>> iova_domain *iovad,
>>> unsigned long flags;
>>> unsigned long new_pfn;
>>> unsigned long align_mask = ~0UL;
>>> + bool retry = false;
>>> if (size_aligned)
>>> align_mask <<= fls_long(size - 1);
>>> @@ -198,6 +199,8 @@ static int __alloc_and_insert_iova_range(struct
>>> iova_domain *iovad,
>>> curr = __get_cached_rbnode(iovad, limit_pfn);
>>> curr_iova = rb_entry(curr, struct iova, node);
>>> +
>>> +retry_search:
>>> do {
>>> limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
>>> new_pfn = (limit_pfn - size) & align_mask;
>>> @@ -207,6 +210,14 @@ static int __alloc_and_insert_iova_range(struct
>>> iova_domain *iovad,
>>> } while (curr && new_pfn <= curr_iova->pfn_hi);
>>> if (limit_pfn < size || new_pfn < iovad->start_pfn) {
>>> + if (!retry) {
>>> + curr = rb_last(>rbroot);
>>
>> Why walk when there's an anchor node there already? However...
>>
>>> + curr_iova = rb_entry(curr, struct iova, node);
>>> + limit_pfn = curr_iova->pfn_lo;
>>
>> ...this doesn't look right, as by now we've lost the original limit_pfn
>> supplied by the caller, so are highly likely to allocate beyond the
>> range our caller asked for. In fact AFAICS we'd start allocating from
>> directly directly below the anchor node, beyond the end of the entire
>> address space.
>>
>> The logic I was imagining we want here was something like the rapidly
>> hacked up (and untested) diff below.
>>
>> Thanks,
>> Robin.
>>
>
> Thanks for your comments ,I have gone through below logic and I see some
> issue with retry check as there could be case where alloc_lo is set to
> some pfn other than start_pfn in that case we don't retry and there can
> still be iova available. I understand its a hacked up version, I can
> work on this.
>
> But how about we just store limit_pfn and get the node using that and
> retry for once from that node, it would be similar to my patch just
> correcting the curr node and limit_pfn update in retry check. do you see
> any issue with this approach ?
>
>
> Thanks,
> Vijay.
I found one issue with my earlier approach, where we search twice from
cached node to the start_pfn, this can be avoided if we store the pfn_hi
of the cached node make this as alloc_lo when we retry. I see the below
diff also does the same, I have posted v2 version of the
Re: [PATCH] iommu/iova: Retry from last rb tree node if iova search fails
On 5/7/2020 6:54 PM, Robin Murphy wrote:
> On 2020-05-06 9:01 pm, vji...@codeaurora.org wrote:
>> From: Vijayanand Jitta
>>
>> When ever a new iova alloc request comes iova is always searched
>> from the cached node and the nodes which are previous to cached
>> node. So, even if there is free iova space available in the nodes
>> which are next to the cached node iova allocation can still fail
>> because of this approach.
>>
>> Consider the following sequence of iova alloc and frees on
>> 1GB of iova space
>>
>> 1) alloc - 500MB
>> 2) alloc - 12MB
>> 3) alloc - 499MB
>> 4) free - 12MB which was allocated in step 2
>> 5) alloc - 13MB
>>
>> After the above sequence we will have 12MB of free iova space and
>> cached node will be pointing to the iova pfn of last alloc of 13MB
>> which will be the lowest iova pfn of that iova space. Now if we get an
>> alloc request of 2MB we just search from cached node and then look
>> for lower iova pfn's for free iova and as they aren't any, iova alloc
>> fails though there is 12MB of free iova space.
>
> Yup, this could definitely do with improving. Unfortunately I think this
> particular implementation is slightly flawed...
>
>> To avoid such iova search failures do a retry from the last rb tree node
>> when iova search fails, this will search the entire tree and get an iova
>> if its available
>>
>> Signed-off-by: Vijayanand Jitta
>> ---
>> drivers/iommu/iova.c | 11 +++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
>> index 0e6a953..2985222 100644
>> --- a/drivers/iommu/iova.c
>> +++ b/drivers/iommu/iova.c
>> @@ -186,6 +186,7 @@ static int __alloc_and_insert_iova_range(struct
>> iova_domain *iovad,
>> unsigned long flags;
>> unsigned long new_pfn;
>> unsigned long align_mask = ~0UL;
>> + bool retry = false;
>> if (size_aligned)
>> align_mask <<= fls_long(size - 1);
>> @@ -198,6 +199,8 @@ static int __alloc_and_insert_iova_range(struct
>> iova_domain *iovad,
>> curr = __get_cached_rbnode(iovad, limit_pfn);
>> curr_iova = rb_entry(curr, struct iova, node);
>> +
>> +retry_search:
>> do {
>> limit_pfn = min(limit_pfn, curr_iova->pfn_lo);
>> new_pfn = (limit_pfn - size) & align_mask;
>> @@ -207,6 +210,14 @@ static int __alloc_and_insert_iova_range(struct
>> iova_domain *iovad,
>> } while (curr && new_pfn <= curr_iova->pfn_hi);
>> if (limit_pfn < size || new_pfn < iovad->start_pfn) {
>> + if (!retry) {
>> + curr = rb_last(>rbroot);
>
> Why walk when there's an anchor node there already? However...
>
>> + curr_iova = rb_entry(curr, struct iova, node);
>> + limit_pfn = curr_iova->pfn_lo;
>
> ...this doesn't look right, as by now we've lost the original limit_pfn
> supplied by the caller, so are highly likely to allocate beyond the
> range our caller asked for. In fact AFAICS we'd start allocating from
> directly directly below the anchor node, beyond the end of the entire
> address space.
>
> The logic I was imagining we want here was something like the rapidly
> hacked up (and untested) diff below.
>
> Thanks,
> Robin.
>
Thanks for your comments ,I have gone through below logic and I see some
issue with retry check as there could be case where alloc_lo is set to
some pfn other than start_pfn in that case we don't retry and there can
still be iova available. I understand its a hacked up version, I can
work on this.
But how about we just store limit_pfn and get the node using that and
retry for once from that node, it would be similar to my patch just
correcting the curr node and limit_pfn update in retry check. do you see
any issue with this approach ?
Thanks,
Vijay.
> ->8-
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 0e6a9536eca6..3574c19272d6 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -186,6 +186,7 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
> unsigned long flags;
> unsigned long new_pfn;
> unsigned long align_mask = ~0UL;
> + unsigned long alloc_hi, alloc_lo;
>
> if (size_aligned)
> align_mask <<= fls_long(size - 1);
> @@ -196,17 +197,27 @@ static int __alloc_and_insert_iova_range(struct
> iova_domain *iovad,
> size >= iovad->max32_alloc_size)
> goto iova
