Re: [PATCH] leds: trigger: cpu: use cpuhp_setup_state_nocalls() for large CPU counts
Lee Jones <[email protected]> Thu, 6 Aug 2026 15:15:13 +0100
| Newsgroups | org.kernel.vger.linux-leds,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 27 Jul 2026, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > - [High] Skipping hotplug startup callbacks during initialization leaves the CPU LED triggers in an incorrect inactive state for all already-online CPUs. > - [Low] The commit message description is not capitalized, violating LED subsystem conventions. Please review, explain or fix the Sashiko review(s). If this is a set, this request includes the other patches too. > -- > > --- Patch [1]: [PATCH] leds: trigger: cpu: use cpuhp_setup_state_nocalls() for large CPU counts --- > commit 2aa3e021f1cce38c0cc7487063d486ab550d6162 > Author: Paul Sherman <[email protected]> > > leds: trigger: cpu: use cpuhp_setup_state_nocalls() for large CPU counts > > This commit changes ledtrig_cpu_init() to use cpuhp_setup_state_nocalls() > instead of cpuhp_setup_state(). This avoids invoking the hotplug callbacks for > already-online CPUs at registration time to prevent boot delays on systems > with large CPU counts. > > > leds: trigger: cpu: use cpuhp_setup_state_nocalls() for large CPU counts > > [Severity: Low] > This isn't a bug, but the commit message description starts with a lowercase > letter. The LED subsystem guidelines mandate: "Always capitalize the description > after the subsystem prefix for MFD, LED, and Backlight subsystems." > > > diff --git a/drivers/leds/trigger/ledtrig-cpu.c b/drivers/leds/trigger/ledtrig-cpu.c > > index 679323c2ccda0..ce75d5f54684c 100644 > > --- a/drivers/leds/trigger/ledtrig-cpu.c > > +++ b/drivers/leds/trigger/ledtrig-cpu.c > > @@ -163,7 +163,15 @@ static int __init ledtrig_cpu_init(void) > > > > register_syscore(&ledtrig_cpu_syscore); > > > > - ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "leds/trigger:starting", > > + /* > > + * Use cpuhp_setup_state_nocalls() to avoid invoking ledtrig_online_cpu() > > + * on every already-online CPU at registration time. On systems with large > > + * CPU counts (e.g. 64-hart RISC-V) cpuhp_setup_state() would serialize > > + * callbacks across all online CPUs, causing multi-hundred-second delays > > + * during boot. The trigger operates correctly for future hotplug events > > + * regardless. > > + */ > > + ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "leds/trigger:starting", > > + ledtrig_online_cpu, ledtrig_prepare_down_cpu); > > [Severity: High] > Does skipping the hotplug startup callbacks during initialization leave the CPU > LED triggers in an incorrect inactive state for all already-online CPUs? > > By replacing cpuhp_setup_state() with cpuhp_setup_state_nocalls(), the driver > skips invoking ledtrig_online_cpu() for already-online CPUs during registration. > Because the cpu_trig per-CPU variables and num_active_cpus are zero-initialized, > they remain in the inactive state. > > On architectures that do not emit ledtrig_cpu events during idle transitions, > these CPUs will never call ledtrig_cpu(CPU_LED_START), so their LEDs will never > turn on unless they undergo a hotplug offline/online cycle. > > -- > Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1 -- Lee Jones