From patchwork Fri Apr 17 15:36:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Filipe Manana X-Patchwork-Id: 11495433 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6973A15AB for ; Fri, 17 Apr 2020 15:36:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 521A921924 for ; Fri, 17 Apr 2020 15:36:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1587137779; bh=B3pkrIA5dZ68hPeqXsUjRMoNzibxzZpcV1uZE0ZNOFY=; h=From:To:Subject:Date:In-Reply-To:References:List-ID:From; b=c4wQJfv9tiYJZKhMij0XkoMp+vl0e4I7cki5cElf2IL5+PaoXvhUWTwsuh5cWhqw7 s2ccI8zgJVbABEprgFTSgFDPoOH+yOCrAME9tjlmsPThXrcJhe9rp/ebu7Pt9NVa2q oBR2jUFWUFEsSl4tq1XBtPDaZ6ESdvIi6KfiQbVY= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729131AbgDQPgS (ORCPT ); Fri, 17 Apr 2020 11:36:18 -0400 Received: from mail.kernel.org ([198.145.29.99]:53740 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728956AbgDQPgS (ORCPT ); Fri, 17 Apr 2020 11:36:18 -0400 Received: from debian7.Home (bl8-197-74.dsl.telepac.pt [85.241.197.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 87A5820776 for ; Fri, 17 Apr 2020 15:36:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1587137778; bh=B3pkrIA5dZ68hPeqXsUjRMoNzibxzZpcV1uZE0ZNOFY=; h=From:To:Subject:Date:In-Reply-To:References:From; b=rzaZawIK9qblPkGugvtzH2klVseX1WzfSqGUMQMVaWbaSjT3wQfJdqSr7wPnhKhp2 E7t8dlBQU3Vf8EJzapMOqBHUPSCguFgx6lMflXixiW1eX2ubfDSEdm0A0CIOL5Cnmo ls0ZfL04ifFfIlrP+uviZdKxR3tGNwSmSTX68qHY= From: fdmanana@kernel.org To: linux-btrfs@vger.kernel.org Subject: [PATCH v2 1/2] Btrfs: fix memory leak of transaction when deleting unused block group Date: Fri, 17 Apr 2020 16:36:15 +0100 Message-Id: <20200417153615.23832-1-fdmanana@kernel.org> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20200417144012.9269-1-fdmanana@kernel.org> References: <20200417144012.9269-1-fdmanana@kernel.org> Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org From: Filipe Manana When cleaning pinned extents right before deleting an unused block group, we check if there's still a previous transaction running and if so we increment its reference count before using it for cleaning pinned ranges in its pinned extents iotree. However we ended up never decrementing the reference count after using the transaction, resulting in a memory leak. Fix it by decrementing the reference count. Signed-off-by: Filipe Manana --- V2: Use btrfs_put_transaction() and not refcount_dec(), otherwise we are not really releasing the memory used by the transaction in case its refcount is 1. Stupid mistake. fs/btrfs/block-group.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index 47f66c6a7d7f..af9e9a008724 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -1288,11 +1288,15 @@ static bool clean_pinned_extents(struct btrfs_trans_handle *trans, if (ret) goto err; mutex_unlock(&fs_info->unused_bg_unpin_mutex); + if (prev_trans) + btrfs_put_transaction(prev_trans); return true; err: mutex_unlock(&fs_info->unused_bg_unpin_mutex); + if (prev_trans) + btrfs_put_transaction(prev_trans); btrfs_dec_block_group_ro(bg); return false; } From patchwork Fri Apr 17 15:36:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Filipe Manana X-Patchwork-Id: 11495435 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D9D1115AB for ; Fri, 17 Apr 2020 15:36:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C302B20776 for ; Fri, 17 Apr 2020 15:36:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1587137814; bh=K09qhucMcxwzEZyW0I6PKGfgL2FxxEKA8SU+YTyXqGU=; h=From:To:Subject:Date:In-Reply-To:References:List-ID:From; b=kHQmRJFKU7WzENBTG3q1R51BXn6LVmg1kS+YfpITQ0WFkNEuV8wKRGHShRVk4Ej2q 5LU9HSrXyER0fhuwahfEHSBdErAizqnM1wtgIyXWKj8aUqDf53SpjTIUYZGvyJm/Uk xz9xnRl3JSZWUGsMgR0i1ORktvsrPTS2cKyrOMDA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729368AbgDQPgx (ORCPT ); Fri, 17 Apr 2020 11:36:53 -0400 Received: from mail.kernel.org ([198.145.29.99]:53998 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729086AbgDQPgx (ORCPT ); Fri, 17 Apr 2020 11:36:53 -0400 Received: from debian7.Home (bl8-197-74.dsl.telepac.pt [85.241.197.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 4CE2220776 for ; Fri, 17 Apr 2020 15:36:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1587137812; bh=K09qhucMcxwzEZyW0I6PKGfgL2FxxEKA8SU+YTyXqGU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=MWnUw9eEVWEKRgN6/B3p+wTGxfezF1lLJRThpaQ28at5Vo05n8YTZF5rAlC0+ldYJ CE/OHtiQgN//vdNUP77ufDMzvgc7AzYq6p6NlitIXXUm7Ts8fQazHcwCopMMr6hoPJ ySekb/G1RdCxMJHf+hAgi9lav3YmAwsbd8Ws571Y= From: fdmanana@kernel.org To: linux-btrfs@vger.kernel.org Subject: [PATCH v2 2/2] Btrfs: simplify error handling of clean_pinned_extents() Date: Fri, 17 Apr 2020 16:36:50 +0100 Message-Id: <20200417153650.23882-1-fdmanana@kernel.org> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20200417144021.9319-1-fdmanana@kernel.org> References: <20200417144021.9319-1-fdmanana@kernel.org> Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org From: Filipe Manana At clean_pinned_extents(), whether we end up returning success or failure, we pretty much have to do the same things: 1) unlock unused_bg_unpin_mutex 2) decrement reference count on the previous transaction We also call btrfs_dec_block_group_ro() in case of failure, but that is better done in its caller, btrfs_delete_unused_bgs(), since its the caller that calls inc_block_group_ro(), so it should be responsible for the decrement operation, as it is in case any of the other functions it calls fail. So move the call to btrfs_dec_block_group_ro() from clean_pinned_extents() into btrfs_delete_unused_bgs() and unify the error and success return paths for clean_pinned_extents(), reducing duplicated code and making it simpler. Signed-off-by: Filipe Manana --- V2: Updated version after patch 1 in the series changed. fs/btrfs/block-group.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index af9e9a008724..f96ab9d6f3fe 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -1280,25 +1280,17 @@ static bool clean_pinned_extents(struct btrfs_trans_handle *trans, ret = clear_extent_bits(&prev_trans->pinned_extents, start, end, EXTENT_DIRTY); if (ret) - goto err; + goto out; } ret = clear_extent_bits(&trans->transaction->pinned_extents, start, end, EXTENT_DIRTY); - if (ret) - goto err; +out: mutex_unlock(&fs_info->unused_bg_unpin_mutex); if (prev_trans) btrfs_put_transaction(prev_trans); - return true; - -err: - mutex_unlock(&fs_info->unused_bg_unpin_mutex); - if (prev_trans) - btrfs_put_transaction(prev_trans); - btrfs_dec_block_group_ro(bg); - return false; + return ret == 0; } /* @@ -1396,8 +1388,10 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info) * We could have pending pinned extents for this block group, * just delete them, we don't care about them anymore. */ - if (!clean_pinned_extents(trans, block_group)) + if (!clean_pinned_extents(trans, block_group)) { + btrfs_dec_block_group_ro(block_group); goto end_trans; + } /* * At this point, the block_group is read only and should fail