Re: [PATCH RFC 0/4] change sb_writers to use percpu_rw_semaphore

[Dave Chinner <david@fromorbit.com>]

[ Please cc linux-fsdevel@vger.kernel.org on filesystem
infrastructure changes! ]

On Mon, Jul 13, 2015 at 11:25:36PM +0200, Oleg Nesterov wrote:
> Hello,
> 
> Al, Jan, could you comment? I mean the intent, the patches are
> obviously not for inclusion yet.
> 
> We can remove everything from struct sb_writers except frozen
> (which can become a boolean, it seems) and add the array of
> percpu_rw_semaphore's instead.
> 
> __sb_start/end_write() can use percpu_down/up_read(), and
> freeze/thaw_super() can use percpu_down/up_write().
> 
> Why:
> 
> 	- Firstly, __sb_start_write() looks simply buggy. I does
> 	  __sb_end_write() if it sees ->frozen, but if it migrates
> 	  to another CPU before percpu_counter_dec() sb_wait_write()
> 	  can wrongly succeed if there is another task which holds
> 	  the same "semaphore": sb_wait_write() can miss the result
> 	  of the previous percpu_counter_inc() but see the result
> 	  of this percpu_counter_dec().
> 
> 	- This code doesn't look simple. It would be better to rely
> 	  on the generic locking code.
> 
> 	- __sb_start_write() will be a little bit faster, but this
> 	  is minor.
> 
> Todo:
> 
> 	- __sb_start_write(wait => false) always fail.
> 
> 	  Thivial, we already have percpu_down_read_trylock() just
> 	  this patch wasn't merged yet.
> 
> 	- sb_lockdep_release() and sb_lockdep_acquire() play with
> 	  percpu_rw_semaphore's internals.
> 
> 	  Trivial, we need a couple of new helper in percpu-rwsem.c.

	- try compiling XFS, watch it break on freeze lockdep
	  annotations

> 	- Fix get_super_thawed(), it will spin if MS_RDONLY...
> 
> 	  It is not clear to me what exactly should we do, but this
> 	  doesn't look hard. Perhaps it can just return if MS_RDONLY.
> 
> 	- Most probably I missed something else, and I do not need
> 	  how to test.

xfstests has many freeze related stress tests.  IIRC, generic/068 is
the test that historically causes the most problems for freeze
infrastructure changes. You'll also need to test at least ext4, XFS
and btrfs, because they all stress the freeze code differently.
Testing XFS, in particular, is a good idea because it has several
custom freeze tests that aren't run on any other filesystem type.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com