On Sun, 2008-06-08 at 22:37 -0400, Chris Mason wrote:
> On Thu, 05 Jun 2008 13:43:48 -0400
> Ric Wheeler <rwheeler@xxxxxxxxxx> wrote:
>
> > Chris Mason wrote:
> > > On Mon, Jun 02, 2008 at 01:52:47PM -0400, Ric Wheeler wrote:
> > >
> > >> I can reliably get btrfs to panic by running my fs_mark code on a
> > >> newly created file system with lots of threads on an 8-way box. If
> > >> this is too aggressive, let me know ;-)
> > >>
> > >> Here is a summary of the panic:
> > >>
> > >
> > > BTW, exactly how are you running fs_mark? Mingming reminded me that
> > > strictly speaking this patch shouldn't be required, so there might
> > > be other related problems.
> > >
> > > -chris
> > >
> > >
> > It still crashes, Mingming is clearly correct ;-)
> >
>
> Grin, I never should have doubted her.
>
:)
> So, the actual fix should be below. It looks like the problem is that I've got
> a race in setting the pointer to a new transaction, which makes the
> data=ordered code take a spin lock that hasn't yet been setup.
>
Just to be clear, so the data=ordered code(btrfs_del_ordered_inode())
takes a spin lock (new_trans_lock) and assume the new transaction has
been setup, that races with join_transaction resetting the current
running transaction()?
I also see the btrfs_commit_transaction() could reset the
root->fs_info->running_transaction to be NULL, but we did not check NULL
pointer in the data=ordered mode code, is this a potential Bug? Or it is
covered somewhere else?
Mingming
> Before this patch my test box got into an infinite loop with fs_mark. Now it
> seems to run to completion.
>
> -chris
>
> diff -r 0b4ab489ffe1 transaction.c
> --- a/transaction.c Tue May 27 10:55:43 2008 -0400
> +++ b/transaction.c Sun Jun 08 22:23:50 2008 -0400
> @@ -56,7 +56,6 @@ static noinline int join_transaction(str
> total_trans++;
> BUG_ON(!cur_trans);
> root->fs_info->generation++;
> - root->fs_info->running_transaction = cur_trans;
> root->fs_info->last_alloc = 0;
> root->fs_info->last_data_alloc = 0;
> cur_trans->num_writers = 1;
> @@ -74,6 +73,9 @@ static noinline int join_transaction(str
> extent_io_tree_init(&cur_trans->dirty_pages,
> root->fs_info->btree_inode->i_mapping,
> GFP_NOFS);
> + spin_lock(&root->fs_info->new_trans_lock);
> + root->fs_info->running_transaction = cur_trans;
> + spin_unlock(&root->fs_info->new_trans_lock);
> } else {
> cur_trans->num_writers++;
> cur_trans->num_joined++;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
