From patchwork Fri Jun 10 18:14:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sage Weil X-Patchwork-Id: 870162 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p5AID5C8005421 for ; Fri, 10 Jun 2011 18:13:05 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757847Ab1FJSMm (ORCPT ); Fri, 10 Jun 2011 14:12:42 -0400 Received: from cobra.newdream.net ([66.33.216.30]:44566 "EHLO cobra.newdream.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757835Ab1FJSMl (ORCPT ); Fri, 10 Jun 2011 14:12:41 -0400 Received: from cobra.newdream.net (localhost [127.0.0.1]) by cobra.newdream.net (Postfix) with ESMTP id 8FCC7BC7B8; Fri, 10 Jun 2011 11:14:42 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=newdream.net; h=date:from:to:cc :subject:in-reply-to:message-id:references:mime-version: content-type; q=dns; s=newdream.net; b=hQ/rV822CF9vQprDEImGWgt+G KXiWygVN7lOkhquS7Cvjzsbm9x4blfLigTQOdwfNCUdkwAs0/8mzh2TVe/Yg/kp9 LryjEn0OFpWkaMR/ZNLdkU98GfJYH56cojMHZoWyfBzHJbyAvve6FS36Nspxgun4 rdvZtkITuqhiJSQ9jk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=newdream.net; h=date:from :to:cc:subject:in-reply-to:message-id:references:mime-version: content-type; s=newdream.net; bh=kbn6GbftHKr5k1kCt6Sb6cB9rj0=; b=dfV2L+glNHNuPLnjJUpG7viOjmH7kcs7dImS6c9yy3HDhFxB2w2dB1lbuNKf7 8epnbF9Us0F2nDcA2rgvOCDBcZHVlDPSIXUjSirOTI+yZplv9ERykOGfzlFRUaYu Oxc3kkak6QaAijmfAVEQ+m3SZP2pt4pbi06LxaiZIIs3D0= Received: by cobra.newdream.net (Postfix, from userid 1031) id 662B3BC7C5; Fri, 10 Jun 2011 11:14:42 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by cobra.newdream.net (Postfix) with ESMTP id 50909BC7B8; Fri, 10 Jun 2011 11:14:42 -0700 (PDT) Date: Fri, 10 Jun 2011 11:14:42 -0700 (PDT) From: Sage Weil To: Chris Mason cc: Jim Schutt , dave , miaox , linux-kernel , linux-btrfs , ceph-devel@vger.kernel.org Subject: Re: 3.0-rcX BUG at fs/btrfs/ioctl.c:432 - bisected In-Reply-To: Message-ID: References: <4DF140AB.4030805@sandia.gov> <20110609234553.GN12709@twin.jikos.cz> <4DF24F0E.4040306@sandia.gov> <1307726732-sup-3404@shiny> MIME-Version: 1.0 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Fri, 10 Jun 2011 18:13:05 +0000 (UTC) On Fri, 10 Jun 2011, Sage Weil wrote: > On Fri, 10 Jun 2011, Chris Mason wrote: > > Excerpts from Jim Schutt's message of 2011-06-10 13:06:22 -0400: > > > > [ two different btrfs crashes ] > > > > I think your two crashes in btrfs were from the uninit variables and > > those should be fixed in rc2. > > > > > When I did my bisection, my criteria for success/failure was > > > "did mkcephfs succeed?". When I apply this criteria to a recent > > > linus kernel (e.g. 06e86849cf4019), which includes the fix you > > > mentioned (aa0467d8d2a00e), I get still a different failure mode, > > > which doesn't actually reference btrfs: > > > > > > [ 276.364178] BUG: unable to handle kernel NULL pointer dereference at 000000000000000a > > > [ 276.365127] IP: [] journal_start+0x3e/0x9c [jbd] > > > > Looking at the resulting code in the oops, we're here in journal_start: > > > > if (handle) { > > J_ASSERT(handle->h_transaction->t_journal == journal); > > > > handle comes from current->journal_info, and we're doing a deref on > > handle->h_transaction, which is probably 0xa. > > > > So, we're leaving crud in current->journal_info and ext3 is finding it. > > > > Perhaps its from ceph starting a transaction but leaving it running? > > The bug came with Josef's transaction performance fixes, but it is > > probably a mixture of his code with the ioctls ceph is using. > > Ah, yeah, that's the problem. We saw a similar problem a while back with > the start/stop transaction ioctls. In this case, create_snapshot is doing > > trans = btrfs_start_transaction(root->fs_info->extent_root, 5); > if (IS_ERR(trans)) { > ret = PTR_ERR(trans); > goto fail; > } > > which sets current->journal_info. Then > > ret = btrfs_snap_reserve_metadata(trans, pending_snapshot); > BUG_ON(ret); > > list_add(&pending_snapshot->list, > &trans->transaction->pending_snapshots); > if (async_transid) { > *async_transid = trans->transid; > ret = btrfs_commit_transaction_async(trans, > root->fs_info->extent_root, 1); > } else { > ret = btrfs_commit_transaction(trans, > root->fs_info->extent_root); > } > > but the async snap creation ioctl takes the async path, which runs > btrfs_commit_transaction in a worker thread. > > I'm not sure what the right thing to do is here is... can whatever is in > journal_info be attached to trans instead in > btrfs_commit_transaction_async()? It looks like it's not used for anything in btrfs, actually; it's just set and cleared. What's the point of that? Anyway, assuming it's useful, I think the below would fix the problem. Want to give it a shot, Jim? Thanks! sage the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index c571734..fd04ad7 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -1196,6 +1196,9 @@ int btrfs_commit_transaction_async(struct btrfs_trans_handle *trans, put_transaction(cur_trans); mutex_unlock(&root->fs_info->trans_mutex); + if (current->journal_info == trans) + current->journal_info = NULL; + return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in