[PATCH] Missing memory barrier in pte_chain_unlock

[PATCH] Missing memory barrier in pte_chain_unlock

[Anton Blanchard]

Hi,

On a ppc64 machine running 2.5.28 we were hitting this BUG in
__free_pages_ok:

BUG_ON(page->pte.chain != NULL);

In pte_chain_lock we use test_and_set_bit which implies a memory
barrier. In pte_chain_unlock we use clear_bit which has no memory
barriers so we need to add one.

Anton

===== include/linux/page-flags.h 1.12 vs edited =====
--- 1.12/include/linux/page-flags.h	Wed Jul 17 07:46:30 2002
+++ edited/include/linux/page-flags.h	Thu Jul 25 19:24:52 2002
@@ -249,6 +248,7 @@
 
 static inline void pte_chain_unlock(struct page *page)
 {
+	smp_mb__before_clear_bit();
 	clear_bit(PG_chainlock, &page->flags);
 	preempt_enable();
 }

Re: [PATCH] Missing memory barrier in pte_chain_unlock

[Andrew Morton]

Anton Blanchard wrote:
> 
> ...
> +       smp_mb__before_clear_bit();
>         clear_bit(PG_chainlock, &page->flags);

Bah.   The problem with this smp_mb thing is that nobody knows
what it does, nobody remembers to do it and it's as ugly as sin.

I bet there are plenty of identical bugs around the place which
haven't been discovered yet.

Is there some clean, centralised way of fixing this problem
permanently?

Correctness comes first.  Why not move the barrier into
clear_bit() and then have a clear_bit_no_mb() operation for those
performance-sensitive places where the barrier is not needed?

 
-