On Mon, 2008-06-09 at 20:47 -0400, Chris Mason wrote: > On Mon, 2008-06-09 at 17:10 -0700, Mingming Cao wrote: > > 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()? > > > Yes > > > 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? > > > > Thanks for double checking these. > > We don't check it in btrfs_add_ordered_inode because that must be called > with the transaction running. > Thanks for clarifying, I missed this. > btrfs_ordered_throttle is safe because it doesn't actually deref the > pointer, it just checks for changes to it. The important part of > ordered_throttle is the writeback count. > > So, the others should be safe, but please let me know if you see any > holes there. > Looks pretty safe to me now, I should not doubt you earlier:) Mingming -- 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
