mm/page_alloc: replace local_lock with normal spinlock -fix -fix

pcpu_spin_unlock and pcpu_spin_unlock_irqrestore both unlock pcp->lock and then enable preemption. This lacks symmetry against both the pcpu_spin helpers and differs from how local_unlock_* is implemented. While this is harmless, it's unnecessary and it's generally better to unwind locks and preemption state in the reverse order as they were acquired.

This is a fix on top of the mm-unstable patch mm-page_alloc-replace-local_lock-with-normal-spinlock-fix.patch

Signed-off-by Mel Gorman <[email protected]>

---
 mm/page_alloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/page_alloc.c +2 -2

index 934d1b5a5449..d0141e51e613 100644
@@ -192,14 +192,14 @@ static DEFINE_MUTEX(pcp_batch_high_lock);
 
 #define pcpu_spin_unlock(member, ptr)					\
 ({									\
-	spin_unlock(&ptr->member);					\
 	pcpu_task_unpin();						\
+	spin_unlock(&ptr->member);					\
 })
 
 #define pcpu_spin_unlock_irqrestore(member, ptr, flags)			\
 ({									\
-	spin_unlock_irqrestore(&ptr->member, flags);			\
 	pcpu_task_unpin();						\
+	spin_unlock_irqrestore(&ptr->member, flags);			\
 })
 
 /* struct per_cpu_pages specific helpers. */

Quoted 6 lines · Mel Gorman

pcpu_spin_unlock and pcpu_spin_unlock_irqrestore both unlock pcp->lock and then enable preemption. This lacks symmetry against both the pcpu_spin helpers and differs from how local_unlock_* is implemented. While this is harmless, it's unnecessary and it's generally better to unwind locks and preemption state in the reverse order as they were acquired.

Hm I'm confused, it seems it's done in reverse order (which I agree with) before this -fix-fix, but not after it?

before, pcpu_spin_lock() (and variants) do pcpu_task_pin() and then spin_lock() (or variant), and pcpu_spin_unlock() does spin_unlock() and then pcpu_task_unpin(). That seems symmetrical, i.e. reverse order to me? And seems to match what local_lock family does too.