Commit de30ec47302c "Remove unnecessary ->wait.lock serialization when reading completion state" was not correct, without lock/unlock the code like stop_machine_from_inactive_cpu() while (!completion_done()) cpu_relax(); can return before complete() finishes its spin_unlock() which writes to this memory. And spin_unlock_wait(). While at it, change try_wait_for_completion() to use READ_ONCE().
Showing the changes to kernel/sched/completion.c (1 of 1 file). Show all
Unified · Split
| @@ -274,7 +274,7 @@ bool try_wait_for_completion(struct completion *x) · 273 unmodified lines | ||
| 274 | 274 | * first without taking the lock so we can |
| 275 | 275 | * return early in the blocking case. |
| 276 | 276 | */ |
| 277 | − if (!ACCESS_ONCE(x->done)) | |
| 277 | + if (!READ_ONCE(x->done)) | |
| 278 | 278 | return 0; |
| 279 | 279 | |
| 280 | 280 | spin_lock_irqsave(&x->wait.lock, flags); |
| @@ -297,6 +297,21 @@ EXPORT_SYMBOL(try_wait_for_completion); · 16 unmodified lines | ||
| 297 | 297 | */ |
| 298 | 298 | bool completion_done(struct completion *x) |
| 299 | 299 | { |
| 300 | − return !!ACCESS_ONCE(x->done); | |
| 300 | + if (!READ_ONCE(x->done)) | |
| 301 | + return false; | |
| 302 | + | |
| 303 | + /* | |
| 304 | + * If ->done, we need to wait for complete() to release ->wait.lock | |
| 305 | + * otherwise we can end up freeing the completion before complete() | |
| 306 | + * is done referencing it. | |
| 307 | + * | |
| 308 | + * The RMB pairs with complete()'s RELEASE of ->wait.lock and orders | |
| 309 | + * the loads of ->done and ->wait.lock such that we cannot observe | |
| 310 | + * the lock before complete() acquires it while observing the ->done | |
| 311 | + * after it's acquired the lock. | |
| 312 | + */ | |
| 313 | + smp_rmb(); | |
| 314 | + spin_unlock_wait(&x->wait.lock); | |
| 315 | + return true; | |
| 301 | 316 | } |
| 302 | 317 | EXPORT_SYMBOL(completion_done); |