Re: [PATCH] Btrfs: don't clear uptodate if the eb is under IO

[Chris Mason <clm@fb.com>]

On 03/28/2014 05:07 PM, Josef Bacik wrote:
> So I have an awful exercise script that will run snapshot, balance and
> send/receive in parallel.  This sometimes would crash spectacularly and when it
> came back up the fs would be completely hosed.  Turns out this is because of a
> bad interaction of balance and send/receive.  Send will hold onto its entire
> path for the whole send, but its blocks could get relocated out from underneath
> it, and because it doesn't old tree locks theres nothing to keep this from
> happening.  So it will go to read in a slot with an old transid, and we could
> have re-allocated this block for something else and it could have a completely
> different transid.  But because we think it is invalid we clear uptodate and
> re-read in the block.  If we do this before we actually write out the new block
> we could write back stale data to the fs, and boom we're screwed.
>
> Now we definitely need to fix this disconnect between send and balance, but we
> really really need to not allow ourselves to accidently read in stale data over
> new data.  So make sure we check if the extent buffer is not under io before
> clearing uptodate, this will kick back EIO to the caller instead of reading in
> stale data and keep us from corrupting the fs.  Thanks,

Josef is on vacation, so I'm posting a short follow up:

> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 0148999..6f035ee 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -5609,7 +5609,9 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
>   			NULL);
>   	sort_clone_roots = 1;
>
> +	current->journal_info = (void *)BTRFS_SEND_TRANS_STUB;
>   	ret = send_subvol(sctx);
> +	current->journal_info = NULL;
>   	if (ret < 0)
>   		goto out;
>

This confuses start_transaction() which tries to dereference whatever it 
finds in current->journal_info.  It just needs an extra check against 
BTRFS_SEND_TRANS_STUB.

I'm folding the fix into his original patch in my tree, and rerunning 
tests here.

-chris