Re: [PATCH 1/4] ext4: Fix deadlock during page writeback

[Theodore Ts'o <tytso@mit.edu>]

On Wed, Jul 06, 2016 at 02:52:28PM +0200, Jan Kara wrote:
> > Starting another transaction while we are waiting for earlier
> > transaction to lock down is going to be problematic, since while there
> > are still handles active on the first transaction, they could still be
> > modifying metadata blocks.  And while that's happening, we can't allow
> > any new handles associated with the second transaction to start
> > modifying metadata blocks.
> 
> Well, we can. We just have to make sure we snapshot the contents that
> should be committed before we modify it from the new transaction. We
> already do this when we are committing block and need to modify it in the
> running transaction at the same time. Obviously allowing this logic to
> trigger earlier will lead to higher memory overhead and allocation,
> copying, and freeing of block snapshots isn't free either so it will need
> careful benchmarking.

Consider the following sequence:

Start handle A attached to txn #42

            <Start Commiting transaction #42>

	    	   	     		Start handle B attached to tnx #43
					Call get_write_access on block bitmap #100
					Modify block bitmap #100
					journal_dirty_metadata for #100

Call get_write_access on block bitmap #100
Modify block bitmap #100
journal_dirty_metadata for #100


Snapshotting the block bitmap at when handle B calls
get_write_access() won't help, because if handle B starts modifying
the block bitmap, and *then* handle A starts trying to modify the same
block bitmap, what do we do?

You could make handle A make the same logical modification in both the
copy of metadata block associated with first transaction (#42) as well
as the copy of the metadata block associated with the second
transaction (#43), and for an allocation bitmap maybe it's even
doable.

But consider the even more hairy case where handle A and handle B are
both modifying an inline xattr, and handle B has to convert spill some
of the extended attribute contents to an external xattr block.  Now
when handle A makes some other xattr change, the change it needs to
make for transaction #42 might be very different from the one for
transaction #43.

The complexity for handling this would be extremely high, and I
suspect doing a two-pass truncate would actually be simpler....

	      		 	  	- Ted


> > If there was some way for all of the currently open handles to
> > guarantee that they won't call get_write_access() on any new blocks,
> > maybe.  But if you look at truncate for example, that gets messy ---
> > and we could get most of the benefit by simply making truncate be a
> > two part operation, where it identifies all of the blocks it needs to
> > modify and makes sure they are in memory *before* it calls
> > start_this_handle.  And then this falls into the general design
> > principle of keeping the run time of handles as short as possible.
> 
> Yeah, I'm afraid the complexity of this will be rather high...
> 
> 								Honza
> 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR