From patchwork Fri Mar 17 11:08:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 13178898 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DE5F8C6FD1D for ; Fri, 17 Mar 2023 11:08:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229783AbjCQLIb (ORCPT ); Fri, 17 Mar 2023 07:08:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40602 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229707AbjCQLIa (ORCPT ); Fri, 17 Mar 2023 07:08:30 -0400 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 74F1224733 for ; Fri, 17 Mar 2023 04:08:28 -0700 (PDT) Received: by mail-wr1-x434.google.com with SMTP id v16so4083876wrn.0 for ; Fri, 17 Mar 2023 04:08:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679051307; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=h7e309zG3GVSY+X1IZJ2K6Xi6wPSGKzC+UwjI2avX+I=; b=OI/BCaicw0Hc3EyJ85lkONU+7IiFOaAmwRld63bIIbePMnc5/D7kAAqsu1Dy1UwBdd wd+RLROJHRlEtRw7livYu7/cOtTNKQ0dXlcSXhYPb+vMefLyD3kck6IkJiG9YZLw6Hmq d74QY10eOhP4iaxFT6Yog9QzdN8NLtnpqVTP61FHE7llwXN8alzxfSfjwL5i4K8jU52w HgQY+jSK54Q4G8lQhV+XzTYI4ZzbxioZFqxzwJoX9br9JRK6SnURTG2WZhA3NXCTdlc8 vp8Ke+RKkvGgS0M1oO8MnvicMMEbjT2skfg89pokQxSSLL7DxQVeuQAZhUVU5LVyuBlJ 6Tcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679051307; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=h7e309zG3GVSY+X1IZJ2K6Xi6wPSGKzC+UwjI2avX+I=; b=6AjRWm8OiKFCqCP+G0+9EtN6G5wKXwAryqnLSBoPVB0RKmjbfvYWiJEyn/xWk4lj2b Dca3LFaXlMVJrk9W+FUQ+C70LFkL0FQSWdkFuvkwN9GvIWj4zqSL0g6p1FuiPPL3JAJN WYRlWniAj9Njj0lW8I833kgjHPd8QxtGIP7p8wi8dWhpRf0g8B4w8VL51MfZCWmFmiYZ GITdxfNf4LCWZtMCOu/FdA67pNeb2ZqwX3S+LO6qY/Tjg52r/LYTbeN/ys1SPaIxl9wn IYTJHmCWJLjSlFD9BsttwYGewNer5T4VstJMOxEogLUsTnb++0P+5q/9IuIR6XfwgWWe eMoA== X-Gm-Message-State: AO0yUKVyQ64/MFYUcBQlpCL0kvVhIZyIlKxXMMRc6OAdaYCJetzxRxgz htpJmLN1/Pjb8o7CK6OOvtc= X-Google-Smtp-Source: AK7set8mtNtIVYvDZW4KuhNNJU/hmSsouIGW4zHuoBGOfgygtedSbzt55bWTg02z88ty0Zxu8SHSrg== X-Received: by 2002:a5d:4e91:0:b0:2d3:9db2:8160 with SMTP id e17-20020a5d4e91000000b002d39db28160mr1117139wru.33.1679051306939; Fri, 17 Mar 2023 04:08:26 -0700 (PDT) Received: from amir-ThinkPad-T480.lan ([5.29.249.86]) by smtp.gmail.com with ESMTPSA id t14-20020a1c770e000000b003daf7721bb3sm7551100wmi.12.2023.03.17.04.08.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Mar 2023 04:08:26 -0700 (PDT) From: Amir Goldstein To: "Darrick J . Wong" Cc: Leah Rumancik , Chandan Babu R , Christian Brauner , linux-xfs@vger.kernel.org, Christoph Hellwig , Dave Chinner , Dave Chinner Subject: [PATCH 5.10 CANDIDATE 02/15] xfs: purge dquots after inode walk fails during quotacheck Date: Fri, 17 Mar 2023 13:08:04 +0200 Message-Id: <20230317110817.1226324-3-amir73il@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230317110817.1226324-1-amir73il@gmail.com> References: <20230317110817.1226324-1-amir73il@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: "Darrick J. Wong" commit 86d40f1e49e9a909d25c35ba01bea80dbcd758cb upstream. [add XFS_QMOPT_QUOTALL flag to xfs_qm_dqpurge_all() for 5.10.y backport] xfs/434 and xfs/436 have been reporting occasional memory leaks of xfs_dquot objects. These tests themselves were the messenger, not the culprit, since they unload the xfs module, which trips the slub debugging code while tearing down all the xfs slab caches: ============================================================================= BUG xfs_dquot (Tainted: G W ): Objects remaining in xfs_dquot on __kmem_cache_shutdown() ----------------------------------------------------------------------------- Slab 0xffffea000606de00 objects=30 used=5 fp=0xffff888181b78a78 flags=0x17ff80000010200(slab|head|node=0|zone=2|lastcpupid=0xfff) CPU: 0 PID: 3953166 Comm: modprobe Tainted: G W 5.18.0-rc6-djwx #rc6 d5824be9e46a2393677bda868f9b154d917ca6a7 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20171121_152543-x86-ol7-builder-01.us.oracle.com-4.el7.1 04/01/2014 Since we don't generally rmmod the xfs module between fstests, this means that xfs/434 is really just the canary in the coal mine -- something leaked a dquot, but we don't know who. After days of pounding on fstests with kmemleak enabled, I finally got it to spit this out: unreferenced object 0xffff8880465654c0 (size 536): comm "u10:4", pid 88, jiffies 4294935810 (age 29.512s) hex dump (first 32 bytes): 60 4a 56 46 80 88 ff ff 58 ea e4 5c 80 88 ff ff `JVF....X..\.... 00 e0 52 49 80 88 ff ff 01 00 01 00 00 00 00 00 ..RI............ backtrace: [] xfs_dquot_alloc+0x2c/0x530 [xfs] [] xfs_qm_dqread+0x6f/0x330 [xfs] [] xfs_qm_dqget+0x132/0x4e0 [xfs] [] xfs_qm_quotacheck_dqadjust+0xa0/0x3e0 [xfs] [] xfs_qm_dqusage_adjust+0x35d/0x4f0 [xfs] [] xfs_iwalk_ag_recs+0x348/0x5d0 [xfs] [] xfs_iwalk_run_callbacks+0x273/0x540 [xfs] [] xfs_iwalk_ag+0x5ed/0x890 [xfs] [] xfs_iwalk_ag_work+0xff/0x170 [xfs] [] xfs_pwork_work+0x79/0x130 [xfs] [] process_one_work+0x672/0x1040 [] worker_thread+0x59b/0xec0 [] kthread+0x29e/0x340 [] ret_from_fork+0x1f/0x30 Now we know that quotacheck is at fault, but even this report was canaryish -- it was triggered by xfs/494, which doesn't actually mount any filesystems. (kmemleak can be a little slow to notice leaks, even with fstests repeatedly whacking it to look for them.) Looking at the *previous* fstest, however, showed that the test run before xfs/494 was xfs/117. The tipoff to the problem is in this excerpt from dmesg: XFS (sda4): Quotacheck needed: Please wait. XFS (sda4): Metadata corruption detected at xfs_dinode_verify.part.0+0xdb/0x7b0 [xfs], inode 0x119 dinode XFS (sda4): Unmount and run xfs_repair XFS (sda4): First 128 bytes of corrupted metadata buffer: 00000000: 49 4e 81 a4 03 02 00 00 00 00 00 00 00 00 00 00 IN.............. 00000010: 00 00 00 01 00 00 00 00 00 90 57 54 54 1a 4c 68 ..........WTT.Lh 00000020: 81 f9 7d e1 6d ee 16 00 34 bd 7d e1 6d ee 16 00 ..}.m...4.}.m... 00000030: 34 bd 7d e1 6d ee 16 00 00 00 00 00 00 00 00 00 4.}.m........... 00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00000050: 00 00 00 02 00 00 00 00 00 00 00 00 96 80 f3 ab ................ 00000060: ff ff ff ff da 57 7b 11 00 00 00 00 00 00 00 03 .....W{......... 00000070: 00 00 00 01 00 00 00 10 00 00 00 00 00 00 00 08 ................ XFS (sda4): Quotacheck: Unsuccessful (Error -117): Disabling quotas. The dinode verifier decided that the inode was corrupt, which causes iget to return with EFSCORRUPTED. Since this happened during quotacheck, it is obvious that the kernel aborted the inode walk on account of the corruption error and disabled quotas. Unfortunately, we neglect to purge the dquot cache before doing that, which is how the dquots leaked. The problems started 10 years ago in commit b84a3a, when the dquot lists were converted to a radix tree, but the error handling behavior was not correctly preserved -- in that commit, if the bulkstat failed and usrquota was enabled, the bulkstat failure code would be overwritten by the result of flushing all the dquots to disk. As long as that succeeds, we'd continue the quota mount as if everything were ok, but instead we're now operating with a corrupt inode and incorrect quota usage counts. I didn't notice this bug in 2019 when I wrote commit ebd126a, which changed quotacheck to skip the dqflush when the scan doesn't complete due to inode walk failures. Introduced-by: b84a3a96751f ("xfs: remove the per-filesystem list of dquots") Fixes: ebd126a651f8 ("xfs: convert quotacheck to use the new iwalk functions") Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig Reviewed-by: Dave Chinner Signed-off-by: Dave Chinner Signed-off-by: Amir Goldstein --- fs/xfs/xfs_qm.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c index 64e5da33733b..3c17e0c0f816 100644 --- a/fs/xfs/xfs_qm.c +++ b/fs/xfs/xfs_qm.c @@ -1318,8 +1318,15 @@ xfs_qm_quotacheck( error = xfs_iwalk_threaded(mp, 0, 0, xfs_qm_dqusage_adjust, 0, true, NULL); - if (error) + if (error) { + /* + * The inode walk may have partially populated the dquot + * caches. We must purge them before disabling quota and + * tearing down the quotainfo, or else the dquots will leak. + */ + xfs_qm_dqpurge_all(mp, XFS_QMOPT_QUOTALL); goto error_return; + } /* * We've made all the changes that we need to make incore. Flush them