drivers/iommu/io-pgtable-arm.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
From: Arnd Bergmann <arnd@arndb.de>
In general a 'struct device' is way too large to be put on the kernel
stack. Apparently something just caused it to grow a slightly larger,
which pushed the arm_lpae_do_selftests() function over the warning
limit in some configurations:
drivers/iommu/io-pgtable-arm.c:1423:19: error: stack frame size (1032) exceeds limit (1024) in 'arm_lpae_do_selftests' [-Werror,-Wframe-larger-than]
1423 | static int __init arm_lpae_do_selftests(void)
| ^
Change the function to use a dynamically allocated platform_device
instead of the on-stack device structure. The device is not actually
registered with the system, but is initialized enough to be used here.
Fixes: ca25ec247aad ("iommu/io-pgtable-arm: Remove iommu_dev==NULL special case")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/iommu/io-pgtable-arm.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 7632c80edea6..9f3bf0b5e8da 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -13,6 +13,7 @@
#include <linux/bitops.h>
#include <linux/io-pgtable.h>
#include <linux/kernel.h>
+#include <linux/platform_device.h>
#include <linux/sizes.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -1433,15 +1434,17 @@ static int __init arm_lpae_do_selftests(void)
};
int i, j, k, pass = 0, fail = 0;
- struct device dev;
+ struct platform_device *pdev;
struct io_pgtable_cfg cfg = {
.tlb = &dummy_tlb_ops,
.coherent_walk = true,
- .iommu_dev = &dev,
};
- /* __arm_lpae_alloc_pages() merely needs dev_to_node() to work */
- set_dev_node(&dev, NUMA_NO_NODE);
+ pdev = platform_device_alloc("io-pgtable-test", 0);
+ if (!pdev)
+ return -ENOMEM;
+
+ cfg.iommu_dev = &pdev->dev;
for (i = 0; i < ARRAY_SIZE(pgsize); ++i) {
for (j = 0; j < ARRAY_SIZE(address_size); ++j) {
@@ -1461,6 +1464,8 @@ static int __init arm_lpae_do_selftests(void)
}
pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
+ platform_device_put(pdev);
+
return fail ? -EFAULT : 0;
}
subsys_initcall(arm_lpae_do_selftests);
--
2.39.5On 11/04/2025 1:54 pm, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> In general a 'struct device' is way too large to be put on the kernel
> stack. Apparently something just caused it to grow a slightly larger,
> which pushed the arm_lpae_do_selftests() function over the warning
> limit in some configurations:
>
> drivers/iommu/io-pgtable-arm.c:1423:19: error: stack frame size (1032) exceeds limit (1024) in 'arm_lpae_do_selftests' [-Werror,-Wframe-larger-than]
> 1423 | static int __init arm_lpae_do_selftests(void)
> | ^
>
> Change the function to use a dynamically allocated platform_device
> instead of the on-stack device structure. The device is not actually
> registered with the system, but is initialized enough to be used here.
>
> Fixes: ca25ec247aad ("iommu/io-pgtable-arm: Remove iommu_dev==NULL special case")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/iommu/io-pgtable-arm.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 7632c80edea6..9f3bf0b5e8da 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -13,6 +13,7 @@
> #include <linux/bitops.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/platform_device.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> @@ -1433,15 +1434,17 @@ static int __init arm_lpae_do_selftests(void)
> };
>
> int i, j, k, pass = 0, fail = 0;
> - struct device dev;
Could we not simply make this static? Per the comment it's only here to
serve a NUMA node lookup buried deep in the pagetable allocator (TBH my
first thought was to just put an int on the stack and contrive a pointer
as the inverse of dev_to_node(), but I decided that would probably be
too contentious...)
> + struct platform_device *pdev;
> struct io_pgtable_cfg cfg = {
> .tlb = &dummy_tlb_ops,
> .coherent_walk = true,
> - .iommu_dev = &dev,
> };
>
> - /* __arm_lpae_alloc_pages() merely needs dev_to_node() to work */
> - set_dev_node(&dev, NUMA_NO_NODE);
> + pdev = platform_device_alloc("io-pgtable-test", 0);
Otherwise, this would seem to be another perfect case for the new
faux_device.
Thanks,
Robin.
> + if (!pdev)
> + return -ENOMEM;
> +
> + cfg.iommu_dev = &pdev->dev;
>
> for (i = 0; i < ARRAY_SIZE(pgsize); ++i) {
> for (j = 0; j < ARRAY_SIZE(address_size); ++j) {
> @@ -1461,6 +1464,8 @@ static int __init arm_lpae_do_selftests(void)
> }
>
> pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
> + platform_device_put(pdev);
> +
> return fail ? -EFAULT : 0;
> }
> subsys_initcall(arm_lpae_do_selftests);
On Fri, Apr 11, 2025, at 15:19, Robin Murphy wrote:
> On 11/04/2025 1:54 pm, Arnd Bergmann wrote:
>> @@ -1433,15 +1434,17 @@ static int __init arm_lpae_do_selftests(void)
>> };
>>
>> int i, j, k, pass = 0, fail = 0;
>> - struct device dev;
>
> Could we not simply make this static? Per the comment it's only here to
> serve a NUMA node lookup buried deep in the pagetable allocator (TBH my
> first thought was to just put an int on the stack and contrive a pointer
> as the inverse of dev_to_node(), but I decided that would probably be
> too contentious...)
A static device would work here, but that has other (small)
downsides:
- static devices are discouraged for any real purpose because
of the problematic lifetime rules. I think Greg would still
want to eliminate these entirely.
- there is slightly more memory usage: the __init function
gets eliminated after boot, while a static allocation says
around. It could perhaps be made __initdata.
- If we ever need anything beyond the NUMA node from it, the
dynamic allocation is probably close enough to make that
work.
>> + struct platform_device *pdev;
>> struct io_pgtable_cfg cfg = {
>> .tlb = &dummy_tlb_ops,
>> .coherent_walk = true,
>> - .iommu_dev = &dev,
>> };
>>
>> - /* __arm_lpae_alloc_pages() merely needs dev_to_node() to work */
>> - set_dev_node(&dev, NUMA_NO_NODE);
>> + pdev = platform_device_alloc("io-pgtable-test", 0);
>
> Otherwise, this would seem to be another perfect case for the new
> faux_device.
Good point, that is clearly better than platform_device in this
case. Shall I send a new version with that?
Arnd
On 11/04/2025 2:44 pm, Arnd Bergmann wrote:
> On Fri, Apr 11, 2025, at 15:19, Robin Murphy wrote:
>> On 11/04/2025 1:54 pm, Arnd Bergmann wrote:
>>> @@ -1433,15 +1434,17 @@ static int __init arm_lpae_do_selftests(void)
>>> };
>>>
>>> int i, j, k, pass = 0, fail = 0;
>>> - struct device dev;
>>
>> Could we not simply make this static? Per the comment it's only here to
>> serve a NUMA node lookup buried deep in the pagetable allocator (TBH my
>> first thought was to just put an int on the stack and contrive a pointer
>> as the inverse of dev_to_node(), but I decided that would probably be
>> too contentious...)
>
> A static device would work here, but that has other (small)
> downsides:
>
> - static devices are discouraged for any real purpose because
> of the problematic lifetime rules. I think Greg would still
> want to eliminate these entirely.
>
> - there is slightly more memory usage: the __init function
> gets eliminated after boot, while a static allocation says
> around. It could perhaps be made __initdata.
>
> - If we ever need anything beyond the NUMA node from it, the
> dynamic allocation is probably close enough to make that
> work.
>
>>> + struct platform_device *pdev;
>>> struct io_pgtable_cfg cfg = {
>>> .tlb = &dummy_tlb_ops,
>>> .coherent_walk = true,
>>> - .iommu_dev = &dev,
>>> };
>>>
>>> - /* __arm_lpae_alloc_pages() merely needs dev_to_node() to work */
>>> - set_dev_node(&dev, NUMA_NO_NODE);
>>> + pdev = platform_device_alloc("io-pgtable-test", 0);
>>
>> Otherwise, this would seem to be another perfect case for the new
>> faux_device.
>
> Good point, that is clearly better than platform_device in this
> case. Shall I send a new version with that?
Sure, I'm happy to consciously err on the side of caution and
robustness, just making sure :)
Thanks,
Robin.
© 2016 - 2025 Red Hat, Inc.