* [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: Kevin Hao, Arnd Bergmann, llvm, Nick Desaulniers, linux-kernel, Aneesh Kumar K.V, Nicholas Piggin, Justin Stitt, Naveen N. Rao, linuxppc-dev, Bill Wendling 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: Kevin Hao, Arnd Bergmann, llvm, Nick Desaulniers, linux-kernel, Aneesh Kumar K.V, Nicholas Piggin, Justin Stitt, Naveen N. Rao, linuxppc-dev, Bill Wendling 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: llvm, Kevin Hao, Arnd Bergmann, Geoff Levand, Aneesh Kumar K.V, Nick Desaulniers, linux-kernel, Nicholas Piggin, Nathan Chancellor, Naveen N. Rao, Justin Stitt, Geert Uytterhoeven, linuxppc-dev, Bill Wendling 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: llvm, Kevin Hao, Arnd Bergmann, Aneesh Kumar K.V, Nick Desaulniers, linux-kernel, Nicholas Piggin, Nathan Chancellor, Naveen N. Rao, Justin Stitt, Geert Uytterhoeven, linuxppc-dev, Bill Wendling 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: llvm, Kevin Hao, Arnd Bergmann, Aneesh Kumar K.V, Nick Desaulniers, linux-kernel, Nathan Chancellor, Nicholas Piggin, Justin Stitt, Naveen N. Rao, linuxppc-dev, Bill Wendling 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: Nathan Chancellor, Kevin Hao, llvm, Nick Desaulniers, linux-kernel, Nicholas Piggin, Aneesh Kumar K.V, Bill Wendling, Justin Stitt, Naveen N. Rao, linuxppc-dev 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: Nathan Chancellor, Kevin Hao, llvm, Nick Desaulniers, linux-kernel, Nicholas Piggin, Aneesh Kumar K.V, Bill Wendling, Justin Stitt, Naveen N. Rao, linuxppc-dev 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: Kevin Hao, Arnd Bergmann, llvm, Nick Desaulniers, linux-kernel, Aneesh Kumar K.V, Nicholas Piggin, Justin Stitt, Naveen N. Rao, linuxppc-dev, Bill Wendling 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: Kevin Hao, Arnd Bergmann, llvm, Nick Desaulniers, linux-kernel, Aneesh Kumar K.V, Nicholas Piggin, Justin Stitt, Naveen N. Rao, linuxppc-dev, Bill Wendling 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: Kevin Hao, Arnd Bergmann, llvm, Nick Desaulniers, linux-kernel, Aneesh Kumar K.V, Nicholas Piggin, Justin Stitt, Naveen N. Rao, linuxppc-dev, Bill Wendling 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:20 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 NNTP newsgroup(s).