* [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage
@ 2024-03-20 18:03 Arnd Bergmann
2024-03-21 0:03 ` Geoff Levand
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Arnd Bergmann @ 2024-03-20 18:03 UTC (permalink / raw
To: Geoff Levand, Michael Ellerman, Nathan Chancellor, Paul Mackerras,
Geert Uytterhoeven
Cc: Arnd Bergmann, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Nick Desaulniers, Bill Wendling,
Justin Stitt, Kevin Hao, linuxppc-dev, linux-kernel, llvm
From: Arnd Bergmann <arnd@arndb.de>
The device is way too large to be on the stack, causing a warning for
an allmodconfig build with clang:
arch/powerpc/platforms/ps3/device-init.c:771:12: error: stack frame size (2064) exceeds limit (2048) in 'ps3_probe_thread' [-Werror,-Wframe-larger-than]
771 | static int ps3_probe_thread(void *data)
There is only a single thread that ever accesses this, so it's fine to
make it static, which avoids the warning.
Fixes: b4cb2941f855 ("[POWERPC] PS3: Use the HVs storage device notification mechanism properly")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/powerpc/platforms/ps3/device-init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 878bc160246e..ce99f06698a9 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -770,7 +770,7 @@ static struct task_struct *probe_task;
static int ps3_probe_thread(void *data)
{
- struct ps3_notification_device dev;
+ static struct ps3_notification_device dev;
int res;
unsigned int irq;
u64 lpar;
--
2.39.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage
2024-03-20 18:03 [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage Arnd Bergmann
@ 2024-03-21 0:03 ` Geoff Levand
2024-03-21 8:32 ` Geert Uytterhoeven
2024-03-24 1:23 ` [PATCH] powerpc: Fix PS3 allmodconfig warning Geoff Levand
2 siblings, 0 replies; 10+ messages in thread
From: Geoff Levand @ 2024-03-21 0:03 UTC (permalink / raw
To: Arnd Bergmann, Michael Ellerman, Nathan Chancellor,
Paul Mackerras, Geert Uytterhoeven
Cc: Arnd Bergmann, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Nick Desaulniers, Bill Wendling,
Justin Stitt, Kevin Hao, linuxppc-dev, linux-kernel, llvm
On 3/21/24 03:03, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The device is way too large to be on the stack, causing a warning for
> an allmodconfig build with clang:
>
> arch/powerpc/platforms/ps3/device-init.c:771:12: error: stack frame size (2064) exceeds limit (2048) in 'ps3_probe_thread' [-Werror,-Wframe-larger-than]
> 771 | static int ps3_probe_thread(void *data)
>
> There is only a single thread that ever accesses this, so it's fine to
> make it static, which avoids the warning.
>
> Fixes: b4cb2941f855 ("[POWERPC] PS3: Use the HVs storage device notification mechanism properly")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/powerpc/platforms/ps3/device-init.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
> index 878bc160246e..ce99f06698a9 100644
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -770,7 +770,7 @@ static struct task_struct *probe_task;
>
> static int ps3_probe_thread(void *data)
> {
> - struct ps3_notification_device dev;
> + static struct ps3_notification_device dev;
> int res;
> unsigned int irq;
> u64 lpar;
Seems fine.
Acked-by: Geoff Levand <geoff@infradead.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage
2024-03-20 18:03 [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage Arnd Bergmann
2024-03-21 0:03 ` Geoff Levand
@ 2024-03-21 8:32 ` Geert Uytterhoeven
2024-03-21 9:32 ` Geoff Levand
2024-03-22 8:34 ` Geoff Levand
2024-03-24 1:23 ` [PATCH] powerpc: Fix PS3 allmodconfig warning Geoff Levand
2 siblings, 2 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2024-03-21 8:32 UTC (permalink / raw
To: Arnd Bergmann
Cc: Geoff Levand, Michael Ellerman, Nathan Chancellor, Paul Mackerras,
Geert Uytterhoeven, Arnd Bergmann, Nicholas Piggin,
Christophe Leroy, Aneesh Kumar K.V, Naveen N. Rao,
Nick Desaulniers, Bill Wendling, Justin Stitt, Kevin Hao,
linuxppc-dev, linux-kernel, llvm
Hi Arnd,
On Wed, Mar 20, 2024 at 7:03 PM Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The device is way too large to be on the stack, causing a warning for
> an allmodconfig build with clang:
>
> arch/powerpc/platforms/ps3/device-init.c:771:12: error: stack frame size (2064) exceeds limit (2048) in 'ps3_probe_thread' [-Werror,-Wframe-larger-than]
> 771 | static int ps3_probe_thread(void *data)
>
> There is only a single thread that ever accesses this, so it's fine to
> make it static, which avoids the warning.
>
> Fixes: b4cb2941f855 ("[POWERPC] PS3: Use the HVs storage device notification mechanism properly")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks for your patch!
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -770,7 +770,7 @@ static struct task_struct *probe_task;
>
> static int ps3_probe_thread(void *data)
> {
> - struct ps3_notification_device dev;
> + static struct ps3_notification_device dev;
> int res;
> unsigned int irq;
> u64 lpar;
Making it static increases kernel size for everyone. So I'd rather
allocate it dynamically. The thread already allocates a buffer, which
can be replaced at no cost by allocating a structure containing both
the ps3_notification_device and the buffer.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage
2024-03-21 8:32 ` Geert Uytterhoeven
@ 2024-03-21 9:32 ` Geoff Levand
2024-03-22 8:34 ` Geoff Levand
1 sibling, 0 replies; 10+ messages in thread
From: Geoff Levand @ 2024-03-21 9:32 UTC (permalink / raw
To: Geert Uytterhoeven, Arnd Bergmann
Cc: Michael Ellerman, Nathan Chancellor, Paul Mackerras,
Geert Uytterhoeven, Arnd Bergmann, Nicholas Piggin,
Christophe Leroy, Aneesh Kumar K.V, Naveen N. Rao,
Nick Desaulniers, Bill Wendling, Justin Stitt, Kevin Hao,
linuxppc-dev, linux-kernel, llvm
Hi Geert,
On 3/21/24 17:32, Geert Uytterhoeven wrote:
>> static int ps3_probe_thread(void *data)
>> {
>> - struct ps3_notification_device dev;
>> + static struct ps3_notification_device dev;
>> int res;
>> unsigned int irq;
>> u64 lpar;
>
> Making it static increases kernel size for everyone. So I'd rather
> allocate it dynamically. The thread already allocates a buffer, which
> can be replaced at no cost by allocating a structure containing both
> the ps3_notification_device and the buffer.
This seems like a much better solution.
-Geoff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage
2024-03-21 8:32 ` Geert Uytterhoeven
2024-03-21 9:32 ` Geoff Levand
@ 2024-03-22 8:34 ` Geoff Levand
2024-03-22 20:24 ` Arnd Bergmann
1 sibling, 1 reply; 10+ messages in thread
From: Geoff Levand @ 2024-03-22 8:34 UTC (permalink / raw
To: Geert Uytterhoeven, Arnd Bergmann
Cc: Michael Ellerman, Nathan Chancellor, Paul Mackerras,
Arnd Bergmann, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Nick Desaulniers, Bill Wendling,
Justin Stitt, Kevin Hao, linuxppc-dev, linux-kernel, llvm
On 3/21/24 17:32, Geert Uytterhoeven wrote:
> --- a/arch/powerpc/platforms/ps3/device-init.c
>> +++ b/arch/powerpc/platforms/ps3/device-init.c
>> @@ -770,7 +770,7 @@ static struct task_struct *probe_task;
>>
>> static int ps3_probe_thread(void *data)
>> {
>> - struct ps3_notification_device dev;
>> + static struct ps3_notification_device dev;
>> int res;
>> unsigned int irq;
>> u64 lpar;
>
> Making it static increases kernel size for everyone. So I'd rather
> allocate it dynamically. The thread already allocates a buffer, which
> can be replaced at no cost by allocating a structure containing both
> the ps3_notification_device and the buffer.
Here's what I came up with. It builds for me without warnings.
I haven't tested it yet. A review would be appreciated.
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 878bc160246e..9bb44a6ccdaf 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -770,37 +770,48 @@ static struct task_struct *probe_task;
static int ps3_probe_thread(void *data)
{
- struct ps3_notification_device dev;
+ struct ps3_probe_thread_local {
+ struct ps3_notification_device dev;
+ union {
+ char buf[512];
+ struct ps3_notify_cmd notify_cmd;
+ struct ps3_notify_event notify_event;
+ };
+ };
+ struct ps3_probe_thread_local *local;
+ struct ps3_notification_device *dev;
+ struct ps3_notify_cmd *notify_cmd;
+ struct ps3_notify_event *notify_event;
int res;
unsigned int irq;
u64 lpar;
- void *buf;
- struct ps3_notify_cmd *notify_cmd;
- struct ps3_notify_event *notify_event;
pr_debug(" -> %s:%u: kthread started\n", __func__, __LINE__);
- buf = kzalloc(512, GFP_KERNEL);
- if (!buf)
+ local = kzalloc(sizeof(local), GFP_KERNEL);
+
+ if (!local)
return -ENOMEM;
- lpar = ps3_mm_phys_to_lpar(__pa(buf));
- notify_cmd = buf;
- notify_event = buf;
+ dev = &local->dev;
+ notify_cmd = &local->notify_cmd;
+ notify_event = &local->notify_event;
+
+ lpar = ps3_mm_phys_to_lpar(__pa(&local->notify_cmd));
/* dummy system bus device */
- dev.sbd.bus_id = (u64)data;
- dev.sbd.dev_id = PS3_NOTIFICATION_DEV_ID;
- dev.sbd.interrupt_id = PS3_NOTIFICATION_INTERRUPT_ID;
+ dev->sbd.bus_id = (u64)data;
+ dev->sbd.dev_id = PS3_NOTIFICATION_DEV_ID;
+ dev->sbd.interrupt_id = PS3_NOTIFICATION_INTERRUPT_ID;
- res = lv1_open_device(dev.sbd.bus_id, dev.sbd.dev_id, 0);
+ res = lv1_open_device(dev->sbd.bus_id, dev->sbd.dev_id, 0);
if (res) {
pr_err("%s:%u: lv1_open_device failed %s\n", __func__,
__LINE__, ps3_result(res));
goto fail_free;
}
- res = ps3_sb_event_receive_port_setup(&dev.sbd, PS3_BINDING_CPU_ANY,
+ res = ps3_sb_event_receive_port_setup(&dev->sbd, PS3_BINDING_CPU_ANY,
&irq);
if (res) {
pr_err("%s:%u: ps3_sb_event_receive_port_setup failed %d\n",
@@ -808,11 +819,11 @@ static int ps3_probe_thread(void *data)
goto fail_close_device;
}
- spin_lock_init(&dev.lock);
- rcuwait_init(&dev.wait);
+ spin_lock_init(&dev->lock);
+ rcuwait_init(&dev->wait);
res = request_irq(irq, ps3_notification_interrupt, 0,
- "ps3_notification", &dev);
+ "ps3_notification", &local->dev);
if (res) {
pr_err("%s:%u: request_irq failed %d\n", __func__, __LINE__,
res);
@@ -823,7 +834,7 @@ static int ps3_probe_thread(void *data)
notify_cmd->operation_code = 0; /* must be zero */
notify_cmd->event_mask = 1UL << notify_region_probe;
- res = ps3_notification_read_write(&dev, lpar, 1);
+ res = ps3_notification_read_write(&local->dev, lpar, 1);
if (res)
goto fail_free_irq;
@@ -834,36 +845,36 @@ static int ps3_probe_thread(void *data)
memset(notify_event, 0, sizeof(*notify_event));
- res = ps3_notification_read_write(&dev, lpar, 0);
+ res = ps3_notification_read_write(&local->dev, lpar, 0);
if (res)
break;
pr_debug("%s:%u: notify event type 0x%llx bus id %llu dev id %llu"
" type %llu port %llu\n", __func__, __LINE__,
- notify_event->event_type, notify_event->bus_id,
- notify_event->dev_id, notify_event->dev_type,
- notify_event->dev_port);
+ notify_event->event_type, notify_event->bus_id,
+ notify_event->dev_id, notify_event->dev_type,
+ notify_event->dev_port);
if (notify_event->event_type != notify_region_probe ||
- notify_event->bus_id != dev.sbd.bus_id) {
+ notify_event->bus_id != dev->sbd.bus_id) {
pr_warn("%s:%u: bad notify_event: event %llu, dev_id %llu, dev_type %llu\n",
__func__, __LINE__, notify_event->event_type,
notify_event->dev_id, notify_event->dev_type);
continue;
}
- ps3_find_and_add_device(dev.sbd.bus_id, notify_event->dev_id);
+ ps3_find_and_add_device(dev->sbd.bus_id, notify_event->dev_id);
} while (!kthread_should_stop());
fail_free_irq:
- free_irq(irq, &dev);
+ free_irq(irq, &local->dev);
fail_sb_event_receive_port_destroy:
- ps3_sb_event_receive_port_destroy(&dev.sbd, irq);
+ ps3_sb_event_receive_port_destroy(&dev->sbd, irq);
fail_close_device:
- lv1_close_device(dev.sbd.bus_id, dev.sbd.dev_id);
+ lv1_close_device(dev->sbd.bus_id, dev->sbd.dev_id);
fail_free:
- kfree(buf);
+ kfree(local);
probe_task = NULL;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage
2024-03-22 8:34 ` Geoff Levand
@ 2024-03-22 20:24 ` Arnd Bergmann
2024-03-24 1:19 ` Geoff Levand
0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2024-03-22 20:24 UTC (permalink / raw
To: Geoff Levand, Geert Uytterhoeven, Arnd Bergmann
Cc: llvm, Kevin Hao, Aneesh Kumar K.V, Nick Desaulniers, linux-kernel,
Nathan Chancellor, Nicholas Piggin, Justin Stitt, Naveen N. Rao,
linuxppc-dev, Bill Wendling
On Fri, Mar 22, 2024, at 09:34, Geoff Levand wrote:
> On 3/21/24 17:32, Geert Uytterhoeven wrote:
>> --- a/arch/powerpc/platforms/ps3/device-init.c
>>> +++ b/arch/powerpc/platforms/ps3/device-init.c
>>> @@ -770,7 +770,7 @@ static struct task_struct *probe_task;
>>>
>>> static int ps3_probe_thread(void *data)
>>> {
>>> - struct ps3_notification_device dev;
>>> + static struct ps3_notification_device dev;
>>> int res;
>>> unsigned int irq;
>>> u64 lpar;
>>
>> Making it static increases kernel size for everyone. So I'd rather
>> allocate it dynamically. The thread already allocates a buffer, which
>> can be replaced at no cost by allocating a structure containing both
>> the ps3_notification_device and the buffer.
I didn't think it mattered much, given that you would rarely
have a kernel with PS3 support along with other platforms.
I suppose it does increase the size for a PS3-only kernel
as well, while your version makes it smaller.
> Here's what I came up with. It builds for me without warnings.
> I haven't tested it yet. A review would be appreciated.
It seems a little complicated but looks all correct to
me and reduces both stack and .data size, so
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage
2024-03-22 20:24 ` Arnd Bergmann
@ 2024-03-24 1:19 ` Geoff Levand
0 siblings, 0 replies; 10+ messages in thread
From: Geoff Levand @ 2024-03-24 1:19 UTC (permalink / raw
To: Arnd Bergmann, Geert Uytterhoeven, Arnd Bergmann
Cc: llvm, Kevin Hao, Aneesh Kumar K.V, Nick Desaulniers, linux-kernel,
Nathan Chancellor, Nicholas Piggin, Justin Stitt, Naveen N. Rao,
linuxppc-dev, Bill Wendling
On 3/23/24 05:24, Arnd Bergmann wrote:
> On Fri, Mar 22, 2024, at 09:34, Geoff Levand wrote:
>> On 3/21/24 17:32, Geert Uytterhoeven wrote:
>>> --- a/arch/powerpc/platforms/ps3/device-init.c
>>>> +++ b/arch/powerpc/platforms/ps3/device-init.c
>>>> @@ -770,7 +770,7 @@ static struct task_struct *probe_task;
>>>>
>>>> static int ps3_probe_thread(void *data)
>>>> {
>>>> - struct ps3_notification_device dev;
>>>> + static struct ps3_notification_device dev;
>>>> int res;
>>>> unsigned int irq;
>>>> u64 lpar;
>>>
>>> Making it static increases kernel size for everyone. So I'd rather
>>> allocate it dynamically. The thread already allocates a buffer, which
>>> can be replaced at no cost by allocating a structure containing both
>>> the ps3_notification_device and the buffer.
>
> I didn't think it mattered much, given that you would rarely
> have a kernel with PS3 support along with other platforms.
>
> I suppose it does increase the size for a PS3-only kernel
> as well, while your version makes it smaller.
>
>> Here's what I came up with. It builds for me without warnings.
>> I haven't tested it yet. A review would be appreciated.
>
> It seems a little complicated but looks all correct to
> me and reduces both stack and .data size, so
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Thanks Arnd. I also thought it was a bit too much. I made a simpler
version that I'll post as a separate message.
-Geoff
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] powerpc: Fix PS3 allmodconfig warning
2024-03-20 18:03 [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage Arnd Bergmann
2024-03-21 0:03 ` Geoff Levand
2024-03-21 8:32 ` Geert Uytterhoeven
@ 2024-03-24 1:23 ` Geoff Levand
2024-04-01 7:08 ` [PATCH v2] " Geoff Levand
2 siblings, 1 reply; 10+ messages in thread
From: Geoff Levand @ 2024-03-24 1:23 UTC (permalink / raw
To: Arnd Bergmann, Michael Ellerman, Nathan Chancellor,
Paul Mackerras, Geert Uytterhoeven
Cc: Arnd Bergmann, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Nick Desaulniers, Bill Wendling,
Justin Stitt, Kevin Hao, linuxppc-dev, linux-kernel, llvm
The struct ps3_notification_device in the ps3_probe_thread routine
is too large to be on the stack, causing a warning for an
allmodconfig build with clang.
Change the struct ps3_notification_device from a variable on the stack
to a dynamically allocated variable.
Reported-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Geoff Levand <geoff@infradead.org>
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 878bc160246e..03292869e6a1 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -770,49 +770,51 @@ static struct task_struct *probe_task;
static int ps3_probe_thread(void *data)
{
- struct ps3_notification_device dev;
+ struct {
+ struct ps3_notification_device dev;
+ u8 buf[512];
+ } *local;
+ struct ps3_notify_cmd *notify_cmd;
+ struct ps3_notify_event *notify_event;
int res;
unsigned int irq;
u64 lpar;
- void *buf;
- struct ps3_notify_cmd *notify_cmd;
- struct ps3_notify_event *notify_event;
pr_debug(" -> %s:%u: kthread started\n", __func__, __LINE__);
- buf = kzalloc(512, GFP_KERNEL);
- if (!buf)
+ local = kzalloc(sizeof(local), GFP_KERNEL);
+ if (!local)
return -ENOMEM;
- lpar = ps3_mm_phys_to_lpar(__pa(buf));
- notify_cmd = buf;
- notify_event = buf;
+ lpar = ps3_mm_phys_to_lpar(__pa(&local->buf));
+ notify_cmd = (struct ps3_notify_cmd *)&local->buf;
+ notify_event = (struct ps3_notify_event *)&local->buf;
/* dummy system bus device */
- dev.sbd.bus_id = (u64)data;
- dev.sbd.dev_id = PS3_NOTIFICATION_DEV_ID;
- dev.sbd.interrupt_id = PS3_NOTIFICATION_INTERRUPT_ID;
+ local->dev.sbd.bus_id = (u64)data;
+ local->dev.sbd.dev_id = PS3_NOTIFICATION_DEV_ID;
+ local->dev.sbd.interrupt_id = PS3_NOTIFICATION_INTERRUPT_ID;
- res = lv1_open_device(dev.sbd.bus_id, dev.sbd.dev_id, 0);
+ res = lv1_open_device(local->dev.sbd.bus_id, local->dev.sbd.dev_id, 0);
if (res) {
pr_err("%s:%u: lv1_open_device failed %s\n", __func__,
__LINE__, ps3_result(res));
goto fail_free;
}
- res = ps3_sb_event_receive_port_setup(&dev.sbd, PS3_BINDING_CPU_ANY,
- &irq);
+ res = ps3_sb_event_receive_port_setup(&local->dev.sbd,
+ PS3_BINDING_CPU_ANY, &irq);
if (res) {
pr_err("%s:%u: ps3_sb_event_receive_port_setup failed %d\n",
__func__, __LINE__, res);
goto fail_close_device;
}
- spin_lock_init(&dev.lock);
- rcuwait_init(&dev.wait);
+ spin_lock_init(&local->dev.lock);
+ rcuwait_init(&local->dev.wait);
res = request_irq(irq, ps3_notification_interrupt, 0,
- "ps3_notification", &dev);
+ "ps3_notification", &local->dev);
if (res) {
pr_err("%s:%u: request_irq failed %d\n", __func__, __LINE__,
res);
@@ -823,7 +825,7 @@ static int ps3_probe_thread(void *data)
notify_cmd->operation_code = 0; /* must be zero */
notify_cmd->event_mask = 1UL << notify_region_probe;
- res = ps3_notification_read_write(&dev, lpar, 1);
+ res = ps3_notification_read_write(&local->dev, lpar, 1);
if (res)
goto fail_free_irq;
@@ -834,36 +836,37 @@ static int ps3_probe_thread(void *data)
memset(notify_event, 0, sizeof(*notify_event));
- res = ps3_notification_read_write(&dev, lpar, 0);
+ res = ps3_notification_read_write(&local->dev, lpar, 0);
if (res)
break;
pr_debug("%s:%u: notify event type 0x%llx bus id %llu dev id %llu"
" type %llu port %llu\n", __func__, __LINE__,
- notify_event->event_type, notify_event->bus_id,
- notify_event->dev_id, notify_event->dev_type,
- notify_event->dev_port);
+ notify_event->event_type, notify_event->bus_id,
+ notify_event->dev_id, notify_event->dev_type,
+ notify_event->dev_port);
if (notify_event->event_type != notify_region_probe ||
- notify_event->bus_id != dev.sbd.bus_id) {
+ notify_event->bus_id != local->dev.sbd.bus_id) {
pr_warn("%s:%u: bad notify_event: event %llu, dev_id %llu, dev_type %llu\n",
__func__, __LINE__, notify_event->event_type,
notify_event->dev_id, notify_event->dev_type);
continue;
}
- ps3_find_and_add_device(dev.sbd.bus_id, notify_event->dev_id);
+ ps3_find_and_add_device(local->dev.sbd.bus_id,
+ notify_event->dev_id);
} while (!kthread_should_stop());
fail_free_irq:
- free_irq(irq, &dev);
+ free_irq(irq, &local->dev);
fail_sb_event_receive_port_destroy:
- ps3_sb_event_receive_port_destroy(&dev.sbd, irq);
+ ps3_sb_event_receive_port_destroy(&local->dev.sbd, irq);
fail_close_device:
- lv1_close_device(dev.sbd.bus_id, dev.sbd.dev_id);
+ lv1_close_device(local->dev.sbd.bus_id, local->dev.sbd.dev_id);
fail_free:
- kfree(buf);
+ kfree(local);
probe_task = NULL;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2] powerpc: Fix PS3 allmodconfig warning
2024-03-24 1:23 ` [PATCH] powerpc: Fix PS3 allmodconfig warning Geoff Levand
@ 2024-04-01 7:08 ` Geoff Levand
2024-04-22 8:16 ` Michael Ellerman
0 siblings, 1 reply; 10+ messages in thread
From: Geoff Levand @ 2024-04-01 7:08 UTC (permalink / raw
To: Arnd Bergmann, Michael Ellerman, Nathan Chancellor,
Paul Mackerras
Cc: Arnd Bergmann, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Nick Desaulniers, Bill Wendling,
Justin Stitt, Kevin Hao, linuxppc-dev, linux-kernel, llvm
The struct ps3_notification_device in the ps3_probe_thread routine
is too large to be on the stack, causing a warning for an
allmodconfig build with clang.
Change the struct ps3_notification_device from a variable on the stack
to a dynamically allocated variable.
Reported-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Geoff Levand <geoff@infradead.org>
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 878bc160246e..b18e1c92e554 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -770,49 +770,51 @@ static struct task_struct *probe_task;
static int ps3_probe_thread(void *data)
{
- struct ps3_notification_device dev;
+ struct {
+ struct ps3_notification_device dev;
+ u8 buf[512];
+ } *local;
+ struct ps3_notify_cmd *notify_cmd;
+ struct ps3_notify_event *notify_event;
int res;
unsigned int irq;
u64 lpar;
- void *buf;
- struct ps3_notify_cmd *notify_cmd;
- struct ps3_notify_event *notify_event;
pr_debug(" -> %s:%u: kthread started\n", __func__, __LINE__);
- buf = kzalloc(512, GFP_KERNEL);
- if (!buf)
+ local = kzalloc(sizeof(*local), GFP_KERNEL);
+ if (!local)
return -ENOMEM;
- lpar = ps3_mm_phys_to_lpar(__pa(buf));
- notify_cmd = buf;
- notify_event = buf;
+ lpar = ps3_mm_phys_to_lpar(__pa(&local->buf));
+ notify_cmd = (struct ps3_notify_cmd *)&local->buf;
+ notify_event = (struct ps3_notify_event *)&local->buf;
/* dummy system bus device */
- dev.sbd.bus_id = (u64)data;
- dev.sbd.dev_id = PS3_NOTIFICATION_DEV_ID;
- dev.sbd.interrupt_id = PS3_NOTIFICATION_INTERRUPT_ID;
+ local->dev.sbd.bus_id = (u64)data;
+ local->dev.sbd.dev_id = PS3_NOTIFICATION_DEV_ID;
+ local->dev.sbd.interrupt_id = PS3_NOTIFICATION_INTERRUPT_ID;
- res = lv1_open_device(dev.sbd.bus_id, dev.sbd.dev_id, 0);
+ res = lv1_open_device(local->dev.sbd.bus_id, local->dev.sbd.dev_id, 0);
if (res) {
pr_err("%s:%u: lv1_open_device failed %s\n", __func__,
__LINE__, ps3_result(res));
goto fail_free;
}
- res = ps3_sb_event_receive_port_setup(&dev.sbd, PS3_BINDING_CPU_ANY,
- &irq);
+ res = ps3_sb_event_receive_port_setup(&local->dev.sbd,
+ PS3_BINDING_CPU_ANY, &irq);
if (res) {
pr_err("%s:%u: ps3_sb_event_receive_port_setup failed %d\n",
__func__, __LINE__, res);
goto fail_close_device;
}
- spin_lock_init(&dev.lock);
- rcuwait_init(&dev.wait);
+ spin_lock_init(&local->dev.lock);
+ rcuwait_init(&local->dev.wait);
res = request_irq(irq, ps3_notification_interrupt, 0,
- "ps3_notification", &dev);
+ "ps3_notification", &local->dev);
if (res) {
pr_err("%s:%u: request_irq failed %d\n", __func__, __LINE__,
res);
@@ -823,7 +825,7 @@ static int ps3_probe_thread(void *data)
notify_cmd->operation_code = 0; /* must be zero */
notify_cmd->event_mask = 1UL << notify_region_probe;
- res = ps3_notification_read_write(&dev, lpar, 1);
+ res = ps3_notification_read_write(&local->dev, lpar, 1);
if (res)
goto fail_free_irq;
@@ -834,36 +836,37 @@ static int ps3_probe_thread(void *data)
memset(notify_event, 0, sizeof(*notify_event));
- res = ps3_notification_read_write(&dev, lpar, 0);
+ res = ps3_notification_read_write(&local->dev, lpar, 0);
if (res)
break;
pr_debug("%s:%u: notify event type 0x%llx bus id %llu dev id %llu"
" type %llu port %llu\n", __func__, __LINE__,
- notify_event->event_type, notify_event->bus_id,
- notify_event->dev_id, notify_event->dev_type,
- notify_event->dev_port);
+ notify_event->event_type, notify_event->bus_id,
+ notify_event->dev_id, notify_event->dev_type,
+ notify_event->dev_port);
if (notify_event->event_type != notify_region_probe ||
- notify_event->bus_id != dev.sbd.bus_id) {
+ notify_event->bus_id != local->dev.sbd.bus_id) {
pr_warn("%s:%u: bad notify_event: event %llu, dev_id %llu, dev_type %llu\n",
__func__, __LINE__, notify_event->event_type,
notify_event->dev_id, notify_event->dev_type);
continue;
}
- ps3_find_and_add_device(dev.sbd.bus_id, notify_event->dev_id);
+ ps3_find_and_add_device(local->dev.sbd.bus_id,
+ notify_event->dev_id);
} while (!kthread_should_stop());
fail_free_irq:
- free_irq(irq, &dev);
+ free_irq(irq, &local->dev);
fail_sb_event_receive_port_destroy:
- ps3_sb_event_receive_port_destroy(&dev.sbd, irq);
+ ps3_sb_event_receive_port_destroy(&local->dev.sbd, irq);
fail_close_device:
- lv1_close_device(dev.sbd.bus_id, dev.sbd.dev_id);
+ lv1_close_device(local->dev.sbd.bus_id, local->dev.sbd.dev_id);
fail_free:
- kfree(buf);
+ kfree(local);
probe_task = NULL;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] powerpc: Fix PS3 allmodconfig warning
2024-04-01 7:08 ` [PATCH v2] " Geoff Levand
@ 2024-04-22 8:16 ` Michael Ellerman
0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2024-04-22 8:16 UTC (permalink / raw
To: Arnd Bergmann, Michael Ellerman, Nathan Chancellor,
Paul Mackerras, Geoff Levand
Cc: Arnd Bergmann, Nicholas Piggin, Christophe Leroy,
Aneesh Kumar K.V, Naveen N. Rao, Nick Desaulniers, Bill Wendling,
Justin Stitt, Kevin Hao, linuxppc-dev, linux-kernel, llvm
On Mon, 01 Apr 2024 16:08:31 +0900, Geoff Levand wrote:
> The struct ps3_notification_device in the ps3_probe_thread routine
> is too large to be on the stack, causing a warning for an
> allmodconfig build with clang.
>
> Change the struct ps3_notification_device from a variable on the stack
> to a dynamically allocated variable.
>
> [...]
Applied to powerpc/next.
[1/1] powerpc: Fix PS3 allmodconfig warning
https://git.kernel.org/powerpc/c/bfe51886ca544956eb4ff924d1937ac01d0ca9c8
cheers
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-04-22 8:18 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-20 18:03 [PATCH] powerpc: ps3: mark ps3_notification_device static for stack usage Arnd Bergmann
2024-03-21 0:03 ` Geoff Levand
2024-03-21 8:32 ` Geert Uytterhoeven
2024-03-21 9:32 ` Geoff Levand
2024-03-22 8:34 ` Geoff Levand
2024-03-22 20:24 ` Arnd Bergmann
2024-03-24 1:19 ` Geoff Levand
2024-03-24 1:23 ` [PATCH] powerpc: Fix PS3 allmodconfig warning Geoff Levand
2024-04-01 7:08 ` [PATCH v2] " Geoff Levand
2024-04-22 8:16 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).