From patchwork Sun Apr 19 08:17:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Omar Sandoval X-Patchwork-Id: 6238111 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 24C9ABF4A6 for ; Sun, 19 Apr 2015 08:18:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0534C20445 for ; Sun, 19 Apr 2015 08:18:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C17F520443 for ; Sun, 19 Apr 2015 08:18:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750908AbbDSIRu (ORCPT ); Sun, 19 Apr 2015 04:17:50 -0400 Received: from mail-pa0-f45.google.com ([209.85.220.45]:34789 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750806AbbDSIRp (ORCPT ); Sun, 19 Apr 2015 04:17:45 -0400 Received: by pacyx8 with SMTP id yx8so174016950pac.1 for ; Sun, 19 Apr 2015 01:17:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-type:content-transfer-encoding; bh=taO7I7RUFcDMZFO16HQMvuwr/XyB/W1NuvX4ZCkn97w=; b=ivQp45SFX1rR5/MtxrUW56NF9UxTiGTeW7q//WmhC/vQX0K6eGERNkyMMbgb5EN8lM nCybzVaZ+X99Dq6JDpUiWRSN7ldnx+3zBiXwuvvC2iGnypo+WpLEN41ymHd9sY5lqnom SZsyDPPnEJ73cykfml/QP/qgUd6ii1am2FSV/BijVfhnPaUIvAyMiuy6NcU3gQJIMeii AvHuKrstNm5Ds+arNzcfOGMsP7ZNXqU52b3tHTZ5IedUFEp8mQKzbsrSztPHzFdN+ft6 yicO/Askv5CVF4RlzbL/Gp0jGv/bVTDiIR9sp2UQp3XoIf6VJ2TmSIaMLO/WgOqSGITC NB/Q== X-Gm-Message-State: ALoCoQnabHsJpxbT24vUT9p8k6rIuln1fvRWPK8o5qVsgCGd1Fp+6CMYoCvAjeChUnh+kRujV73U X-Received: by 10.70.64.138 with SMTP id o10mr19094913pds.104.1429431465355; Sun, 19 Apr 2015 01:17:45 -0700 (PDT) Received: from mew.localdomain (c-76-104-211-44.hsd1.wa.comcast.net. [76.104.211.44]) by mx.google.com with ESMTPSA id ej10sm3344187pdb.35.2015.04.19.01.17.44 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sun, 19 Apr 2015 01:17:44 -0700 (PDT) From: Omar Sandoval To: linux-btrfs@vger.kernel.org Cc: Omar Sandoval , Shaohua Li Subject: [PATCH] Revert "btrfs: delete chunk allocation attemp when setting block group ro" Date: Sun, 19 Apr 2015 01:17:05 -0700 Message-Id: X-Mailer: git-send-email 2.3.5 MIME-Version: 1.0 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This reverts commit 2f0810880f082fa8ba66ab2c33b02e4ff9770a5e. This tried to fix the following test case: mkfs.ext4 -F /dev/sda btrfs-convert /dev/sda mount /dev/sda /mnt btrfs device add -f /dev/sdb /mnt btrfs balance start -v -dconvert=raid1 -mconvert=raid1 /mnt Before the reverted commit, this test case failed with ENOSPC because all chunks on the freshly converted filesystem were allocated, although many were empty. The reverted commit removed an allocation attempt in btrfs_set_block_group_ro(), but that fix wasn't right. After the reverted commit, the balance succeeds, but the data/metadata profiles aren't actually updated: # btrfs fi df /mnt Data, single: total=208.00MiB, used=49.48MiB System, single: total=32.00MiB, used=4.00KiB Metadata, single: total=208.00MiB, used=48.00KiB GlobalReserve, single: total=4.00MiB, used=0.00B Indeed, several users reported that this commit caused a regression and that converting the data and metadata profiles no longer works. This is because the chunk allocation in question was where we actually allocated the chunk with the new profile. Not seeing a more obvious fix, let's just revert this. We can work around the ENOSPC in the original test case by just issuing a balance to free up the unused block groups before the conversion, anyways: mkfs.ext4 -F /dev/sda btrfs-convert /dev/sda mount /dev/sda /mnt btrfs balance start -v /mnt btrfs device add -f /dev/sdb /mnt btrfs balance start -v -dconvert=raid1 -mconvert=raid1 /mnt Reported-by: Holger Hoffstätte Cc: Shaohua Li Signed-off-by: Omar Sandoval --- Holger reported this awhile ago and I haven't seen a proper fix make it to the ML. I messed around with it for awhile and came up empty, but I figured I might as well send this in. Applies to v4.0. fs/btrfs/extent-tree.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 8b353ad..6421852 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -8537,6 +8537,14 @@ int btrfs_set_block_group_ro(struct btrfs_root *root, if (IS_ERR(trans)) return PTR_ERR(trans); + alloc_flags = update_block_group_flags(root, cache->flags); + if (alloc_flags != cache->flags) { + ret = do_chunk_alloc(trans, root, alloc_flags, + CHUNK_ALLOC_FORCE); + if (ret < 0) + goto out; + } + ret = set_block_group_ro(cache, 0); if (!ret) goto out; @@ -8547,11 +8555,6 @@ int btrfs_set_block_group_ro(struct btrfs_root *root, goto out; ret = set_block_group_ro(cache, 0); out: - if (cache->flags & BTRFS_BLOCK_GROUP_SYSTEM) { - alloc_flags = update_block_group_flags(root, cache->flags); - check_system_chunk(trans, root, alloc_flags); - } - btrfs_end_transaction(trans, root); return ret; }