* [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