Received: by 2002:a5b:505:0:0:0:0:0 with SMTP id o5csp1068593ybp; Thu, 17 Oct 2019 07:34:38 -0700 (PDT) X-Google-Smtp-Source: APXvYqxHuBPwu2vEzy3IoAHzpDf9/VJ5jsyhKhMAAGQdxwRZvrX8VIwcwF5ICvpJ90CJ88asUSeG X-Received: by 2002:a17:906:6a8e:: with SMTP id p14mr3751328ejr.137.1571322878702; Thu, 17 Oct 2019 07:34:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1571322878; cv=none; d=google.com; s=arc-20160816; b=s/nrjxQWgDu4GYbP8/S7EZPYHQG4wtzKJjpGlR4Y/LacSzu8VG7ce9TvkeXGVTM7ok AlURui1gsg+LQrrq7CYUgpYugThkaeTaKbtD0gZLNGg/hBogEzW1Xz/xLQfniYjpXR7u 18hc1NH1DxcoPnzhmeux7UeeOkyFxOE2HUxvDe6X+QkFLWJvTXKgAL7iwQIviNLJQ4Mb maYM7vwcTTWp1kLB1QH+qICch23ETRzorsiDb1u3C0Iz208TeT0Ls9dKzQ7vgKAtRDRP zOVFWIxQnurbY99EsJP8GzgUF3f2zJa/bAy4bVycXBvEwmMgTGXf/a6OXUCkvO67FdhN KNYA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=6SDZ+MAZTLAT3/k8VJKKhrQuQmVQDba+oU5rfuC1mIg=; b=g/zxXw6EuCLrKR5N5JyL96dOTdrbFeUHAYv2jXkpmVfcM0t5VAIPvFjUUenY9qPsm5 BxN7DvyBvtCMaH/rEiqzUfKCnEC1ugYL996uquiXw2zYmtsQ8JgqJb1aAGa0Xwn6A9f5 i6a1L7mtJix8mFRpLPlTmVy7v1YEoqRhVa7uZftUr1gOYTuni991NZzzC8RuJunI/+nb P4J8EAFMtEcfmNr1iy9jtOxml8xJhUE3AQfF7gJ5A1yOOM/PhMoWjHhompRjTkZxUWYO gKT5IM6zBO68QQf7pQ13MK0ZPucDWdUl3W9OBMVMfKHh09ROoGt9Pr5roUpwlfrXMWx6 /QBQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=S75StnAg; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id h23si1526644ejc.1.2019.10.17.07.34.15; Thu, 17 Oct 2019 07:34:38 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=S75StnAg; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2437935AbfJPV4z (ORCPT + 99 others); Wed, 16 Oct 2019 17:56:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:48094 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406644AbfJPV41 (ORCPT ); Wed, 16 Oct 2019 17:56:27 -0400 Received: from localhost (unknown [192.55.54.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 6A0FB21D7E; Wed, 16 Oct 2019 21:56:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1571262986; bh=qFP1NjO6W8Srt9bBH9rasyAL70V1cecuNsWg7IKsMEo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=S75StnAgxkuU2RBkJ+9IBDM9SUEvP9m/2NnojWmv6M1rSJeddZFhac3odUsC3PvEG FLIxlG+tyxtNXedtFHdVXARkay6tdPqGMlI0rPo5TOX/TQlS394ozvS1OENQzez8Mr XFRVOZoJd5p5pvyj8GedfLdLLicUkdlHGqnfBj2I= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Chris Mason , Filipe Manana , Josef Bacik , David Sterba Subject: [PATCH 4.14 55/65] btrfs: fix incorrect updating of log root tree Date: Wed, 16 Oct 2019 14:51:09 -0700 Message-Id: <20191016214837.671712650@linuxfoundation.org> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191016214756.457746573@linuxfoundation.org> References: <20191016214756.457746573@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Josef Bacik commit 4203e968947071586a98b5314fd7ffdea3b4f971 upstream. We've historically had reports of being unable to mount file systems because the tree log root couldn't be read. Usually this is the "parent transid failure", but could be any of the related errors, including "fsid mismatch" or "bad tree block", depending on which block got allocated. The modification of the individual log root items are serialized on the per-log root root_mutex. This means that any modification to the per-subvol log root_item is completely protected. However we update the root item in the log root tree outside of the log root tree log_mutex. We do this in order to allow multiple subvolumes to be updated in each log transaction. This is problematic however because when we are writing the log root tree out we update the super block with the _current_ log root node information. Since these two operations happen independently of each other, you can end up updating the log root tree in between writing out the dirty blocks and setting the super block to point at the current root. This means we'll point at the new root node that hasn't been written out, instead of the one we should be pointing at. Thus whatever garbage or old block we end up pointing at complains when we mount the file system later and try to replay the log. Fix this by copying the log's root item into a local root item copy. Then once we're safely under the log_root_tree->log_mutex we update the root item in the log_root_tree. This way we do not modify the log_root_tree while we're committing it, fixing the problem. CC: stable@vger.kernel.org # 4.4+ Reviewed-by: Chris Mason Reviewed-by: Filipe Manana Signed-off-by: Josef Bacik Signed-off-by: David Sterba Signed-off-by: Greg Kroah-Hartman --- fs/btrfs/tree-log.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -2729,7 +2729,8 @@ out: * in the tree of log roots */ static int update_log_root(struct btrfs_trans_handle *trans, - struct btrfs_root *log) + struct btrfs_root *log, + struct btrfs_root_item *root_item) { struct btrfs_fs_info *fs_info = log->fs_info; int ret; @@ -2737,10 +2738,10 @@ static int update_log_root(struct btrfs_ if (log->log_transid == 1) { /* insert root item on the first sync */ ret = btrfs_insert_root(trans, fs_info->log_root_tree, - &log->root_key, &log->root_item); + &log->root_key, root_item); } else { ret = btrfs_update_root(trans, fs_info->log_root_tree, - &log->root_key, &log->root_item); + &log->root_key, root_item); } return ret; } @@ -2836,6 +2837,7 @@ int btrfs_sync_log(struct btrfs_trans_ha struct btrfs_fs_info *fs_info = root->fs_info; struct btrfs_root *log = root->log_root; struct btrfs_root *log_root_tree = fs_info->log_root_tree; + struct btrfs_root_item new_root_item; int log_transid = 0; struct btrfs_log_ctx root_log_ctx; struct blk_plug plug; @@ -2901,18 +2903,26 @@ int btrfs_sync_log(struct btrfs_trans_ha goto out; } + /* + * We _must_ update under the root->log_mutex in order to make sure we + * have a consistent view of the log root we are trying to commit at + * this moment. + * + * We _must_ copy this into a local copy, because we are not holding the + * log_root_tree->log_mutex yet. This is important because when we + * commit the log_root_tree we must have a consistent view of the + * log_root_tree when we update the super block to point at the + * log_root_tree bytenr. If we update the log_root_tree here we'll race + * with the commit and possibly point at the new block which we may not + * have written out. + */ btrfs_set_root_node(&log->root_item, log->node); + memcpy(&new_root_item, &log->root_item, sizeof(new_root_item)); root->log_transid++; log->log_transid = root->log_transid; root->log_start_pid = 0; /* - * Update or create log root item under the root's log_mutex to prevent - * races with concurrent log syncs that can lead to failure to update - * log root item because it was not created yet. - */ - ret = update_log_root(trans, log); - /* * IO has been started, blocks of the log tree have WRITTEN flag set * in their headers. new modifications of the log will be written to * new positions. so it's safe to allow log writers to go in. @@ -2932,6 +2942,14 @@ int btrfs_sync_log(struct btrfs_trans_ha mutex_unlock(&log_root_tree->log_mutex); mutex_lock(&log_root_tree->log_mutex); + + /* + * Now we are safe to update the log_root_tree because we're under the + * log_mutex, and we're a current writer so we're holding the commit + * open until we drop the log_mutex. + */ + ret = update_log_root(trans, log, &new_root_item); + if (atomic_dec_and_test(&log_root_tree->log_writers)) { /* * Implicit memory barrier after atomic_dec_and_test