linux-kernel - Re: [PATCH]: Fix SMP-reordering race in mark_buffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0804022238460.25023@artax.karlin.mff.cuni.cz>
Date:	Wed, 2 Apr 2008 23:03:27 +0200 (CEST)
From:	Mikulas Patocka <mikulas@...ax.karlin.mff.cuni.cz>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	viro@...iv.linux.org.uk, linux-kernel@...r.kernel.org
Subject: Re: [PATCH]: Fix SMP-reordering race in mark_buffer_dirty

On Wed, 2 Apr 2008, Linus Torvalds wrote:

> On Wed, 2 Apr 2008, Mikulas Patocka wrote:
> > +	/*
> > +	 * Make sure that the test for buffer_dirty(bh) is not reordered with
> > +	 * previous modifications to the buffer data.
> > +	 * -- mikulas
> > +	 */
> > +	smp_mb();
> >  	WARN_ON_ONCE(!buffer_uptodate(bh));
> >  	if (!buffer_dirty(bh) && !test_set_buffer_dirty(bh))
> 
> At that point, the better patch is to just *remove* the buffer_dirty() 
> test, and rely on the stronger ordering requirements of 
> test_set_buffer_dirty().
> 
> The whole - and only - point of the buffer_dirty() check was to avoid the 
> more expensive test_set_buffer_dirty() call, but it's only more expensive 
> because of the barrier semantics. So if you add a barrier, the point goes 
> away and you should instead remove the optimization.
> 
> (I also seriously doubt you can actually trigger this in real life, but 
> simplifying the code is probably fine regardless).
> 
> 		Linus

I measured it:
On Core2, mfence is faster (8 ticks) than lock btsl (27 ticks)
On Pentium-4-prescott, mfence is 124 ticks and lock btsl is 86 ticks.
On Pentium-4-pre-prescott, mfence wins again (100) and lock btsl is 120.
On Athlon, mfence is 16 ticks and lock btsl is 19 ticks.

So you're right, the gain of mfence is so little that you can remove it 
and use only test_set_buffer_dirty.

I don't know if there are other architectures where smb_mb() would be 
significantly faster than test_and_set_bit.

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/