[Btrfs-devel] [RFC] wait for more writers to join the transaction

[Chris Mason]

On Tue, 7 Aug 2007 18:11:54 -0400
Josef Bacik <jwhiter at redhat.com> wrote:

> Hello,
> 
> Sorry for my long silence, getting all situated took longer than I
> expected.  I just wrote something very quickly to try and make btrfs
> sleep longer if there were writers while we were sleeping during an
> fsync.  Running the fs_mark test, btrfs by itself gets walloped by
> ext3, so I'm ignoring that for now.  With btrfs+my patch we seem to
> get about the same performance, except in the -S2 and -S5 cases,
> which are

fs_mark can test lots of different things.  In the multi-threaded
tests, the big fat fs_mutex in btrfs is going to hurt us quite a lot,
so that part isn't horribly surprising (this is very high on my list to
fix).

How did you mount ext3?  If you're running on sata or IDE drives, make
sure to use mount -o barrier=1.  btrfs does write barriers by default,
so to be fair you have to turn them on in ext3 as well.

> 
>  Sync method: SYSTEM SYNC/SINGLE FSYNC: Issue sync() after main write
> loop and 1 file fsync() per subdirectory

I would just use the default sync method.  ext3 has some code to
basically make the fsync a noop if the transaction that changed the
file last is already committed.  This would be the next thing I ask you
to add to btrfs ;)

> 
> Sync method: POST: Reopen and fsync() each file in order after main
> write loop
> 
> Now I don't think thats good enough so this is going to require more
> work, but I'm also kind of new to journalling stuff so I wanted to
> run this by people to see if I'm going in the right direction.  Might
> as well get put on the right track before I go hacking around at
> things.  Thank you,
> 
> Josef
> 
> diff -r 1765acb6d268 transaction.c
> --- a/transaction.c	Tue Aug 07 16:35:25 2007 -0400
> +++ b/transaction.c	Tue Aug 07 17:29:12 2007 -0400
> @@ -56,6 +56,7 @@ static int join_transaction(struct btrfs
>  		root->fs_info->generation++;
>  		root->fs_info->running_transaction = cur_trans;
>  		cur_trans->num_writers = 0;
> +		cur_trans->writers_while_asleep = 0;
>  		cur_trans->transid = root->fs_info->generation;
>  		init_waitqueue_head(&cur_trans->writer_wait);
>  		init_waitqueue_head(&cur_trans->commit_wait);
> @@ -67,6 +68,7 @@ static int join_transaction(struct btrfs
>  		init_bit_radix(&cur_trans->dirty_pages);
>  	}
>  	cur_trans->num_writers++;
> +	cur_trans->writers_while_asleep++;
>  	return 0;
>  }
>  
> @@ -454,12 +456,16 @@ int btrfs_commit_transaction(struct btrf
>  			mutex_lock(&root->fs_info->trans_mutex);
>  		}
>  	}
> -	while (trans->transaction->num_writers > 1) {
> +	while (trans->transaction->num_writers > 1 ||
> +		trans->transaction->writers_while_asleep > 0) {
> +
>  		WARN_ON(cur_trans != trans->transaction);
> +		
>  		prepare_to_wait(&trans->transaction->writer_wait,
> &wait, TASK_UNINTERRUPTIBLE);
>  		if (trans->transaction->num_writers <= 1)
>  			break;

I think you need the writers_while_asleep check here too (before the
break).

Thanks for trying this out.

-chris