On Fri, 24 Jan 2025 09:38:25 +0000 Liao Chang <liaochang1@xxxxxxxxxx> wrote: > Since clearing a bit in thread_info is an atomic operation, the spinlock > is redundant and can be removed, reducing lock contention is good for > performance. Although this patch is probably fine, the change log suggests a dangerous precedence. Just because clearing a flag is atomic, that alone does not guarantee that it doesn't need spin locks around it. There may be another path that tests the flag within a spin lock, and then does a bunch of work assuming that the flag does not change while it is doing that work. That other path would require a spin lock around the clearing of the flag elsewhere. I don't know this code well enough to know if this has that scenario, and seeing the Acked-by from Oleg, I'm assuming it does not. But in any case, the change log needs to give a better rationale for removing a spin lock than just "clearing a flag atomically doesn't need a spin lock"! -- Steve > > Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> > Acked-by: Oleg Nesterov <oleg@xxxxxxxxxx> > Signed-off-by: Liao Chang <liaochang1@xxxxxxxxxx>
