From mboxrd@z Thu Jan 1 00:00:00 1970 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 4204FC433F5 for ; Fri, 18 Feb 2022 05:49:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230228AbiBRFtd (ORCPT ); Fri, 18 Feb 2022 00:49:33 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:42812 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229850AbiBRFt2 (ORCPT ); Fri, 18 Feb 2022 00:49:28 -0500 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9E6913DA55 for ; Thu, 17 Feb 2022 21:49:12 -0800 (PST) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id BD9062199B for ; Fri, 18 Feb 2022 05:49:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1645163350; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=VOIV0oqDrwUJprf7RpKdO9vFuXU2AbriUlNc5FqsPx0=; b=vINAK4/7QGJnOcUi1HXaHW27trfRYmDXJm/SuDTR6hteBLfvsmjU3iWTFZs0WWzzUgKo3a nSFvlIfvPh2wVahXr0MkHsRStzmjEMqpwgD7bN5Q4atgJ06QqVQa/Rrbdb4QeCGTiF+AjB /5hfLdEe/3zvCtOyohnevSrdSDRMcbI= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id A30F313BF3 for ; Fri, 18 Feb 2022 05:49:09 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 9RYBG1UzD2JaFQAAMHmgww (envelope-from ) for ; Fri, 18 Feb 2022 05:49:09 +0000 From: Qu Wenruo To: linux-btrfs@vger.kernel.org Subject: [PATCH v3 0/9] btrfs: refactor scrub entrances for each profile Date: Fri, 18 Feb 2022 13:48:43 +0800 Message-Id: X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org The branch is based on several recent submitted small cleanups, thus it's better to fetch the branch from github: https://github.com/adam900710/linux/tree/refactor_scrub [CRAP-BUT-IT-WORKS(TM)] Scrub is one of the area we seldom touch because: - It's a mess Just check scrub_stripe() function. It's a function scrubbing a stripe for *all* profiles. It's near 400 lines for a single complex function, with double while() loop and several different jumps inside the loop. Not to mention the lack of comments for various structures. This should and will never happen under our current code standard. - It just works I have hit more than 10 bugs during development, and I just want to give up the refactor, as even the code is crap, it works, passing the existing scrub/replace group. While no matter how small code change I'm doing, it always fails to pass the same tests. [REFACTOR-IDEA] The core idea here, is to get rid of one-fit-all solution for scrub_stripe(). Instead, we explicitly separate the scrub into 3 groups (the idea is from my btrfs-fuse project): - Simple-mirror based profiles This includes SINGLE/DUP/RAID1/RAID1C* profiles. They have no stripe, and their repair is purely mirror based. - Simple-stripe based profiles This includes RAID0/RAID10 profiles. They are just simple stripe (without P/Q nor rotation), with extra mirrors to support their repair. - RAID56 The most complex profiles, they have extra P/Q, and have rotation. [REFACTOR-IMPLEMENTATION] So we have 3 entrances for all those supported profiles: - scrub_simple_mirror() For SINGLE/DUP/RAID1/RAID1C* profiles. Just go through each extent and scrub the extent. - scrub_simple_stripe() For RAID0/RAID10 profiles. Instead we go each data stripe first, then inside each data stripe, we can call scrub_simple_mirror(), since after stripe split, RAID0 is just SINGLE and RAID10 is just RAID1. - scrub_stripe() untouched for RAID56 RAID56 still has all the complex things to do, but they can still be split into two types (already done by the original code) * data stripes They are no different in the verification part, RAID56 is just SINGLE if we ignore the repair path. It's only in repair path that our path divides. So we can reuse scrub_simple_mirror() again. * P/Q stripes They already have a dedicated function handling the case. With all these refactors, although we have several more functions, we get rid of: - A double while () loop - Several jumps inside the double loop - Complex calculation to try to fit all profiles And we get: - Better comments - More dedicated functions - A better basis for further refactors [FUTURE CLEANUPS] - Refactor scrub_pages/scrub_parity/... structures - Further cleanup RAID56 codes Changelog: v2: - Rebased to latest misc-next - Fix several uninitialized variables in the 2nd and 3rd patch This is because @physical, @physical_end and @offset are also used for zoned device sync. Initial those values early to fix the problem. v3: - Add two patches to better split cleanups from refactors One to change the timing of initialization of @physical and @physical_end One to remove dead non-RAID56 branches after making scrub_stripe() to work on RAID56 only. - Fix an unfinished comment in scrub_simple_mirror() Qu Wenruo (9): btrfs: calculate @physical_end using @dev_extent_len directly in scrub_stripe() btrfs: introduce a helper to locate an extent item btrfs: introduce dedicated helper to scrub simple-mirror based range btrfs: introduce dedicated helper to scrub simple-stripe based range btrfs: scrub: cleanup the non-RAID56 branches in scrub_stripe() btrfs: use scrub_simple_mirror() to handle RAID56 data stripe scrub btrfs: refactor scrub_raid56_parity() btrfs: use find_first_extent_item() to replace the open-coded extent item search btrfs: move scrub_remap_extent() call into scrub_extent() with more comments fs/btrfs/scrub.c | 1034 +++++++++++++++++++++++++--------------------- 1 file changed, 556 insertions(+), 478 deletions(-) -- 2.35.1