Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [PATCH v2 1/2] cpuhp: make target_store() a nop when target == state | Date | Wed, 01 Jun 2022 17:39:26 +0100 |
| |
On 01/06/22 11:49, Phil Auld wrote: > On Mon, May 30, 2022 at 01:27:00PM +0100 Valentin Schneider wrote: >> >> Yeah, you could append a simple: >> >> else >> WARN_ON(st->state != target); > > I was thinking more like: > > else > if (st->target != target) st->target = target; > > Since this is a write to the target field and we are not > doing one of the operations that will set target because > state == target we should make sure target == target. Although > that could have its own issues, I suppose. But as I said > fixing the boot cpu should make it much less likely that > st->target != st->state once we have the hotplug lock. > > I don't see how that WARN would ever fire. We're under the lock > and nothing is re-reading the value of st->state anyway. Looks more > like a compiler sanity check :) >
Right, having a warning in there would mostly be to catch unexpected/unintended scenarios like the one you've found for the boot CPU. I'd vote for
WARN("Huh, didn't expect that") + fix st->target
> > Cheers, > Phil > > >> > Maybe I'll include that if/when I have code to keep cpux/online in sync >> > with st->state and cpu_online_mask. >> > > --
| |