From patchwork Tue Jul 24 02:50:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sandeen X-Patchwork-Id: 10541319 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 08D41112E for ; Tue, 24 Jul 2018 02:50:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EB66C28680 for ; Tue, 24 Jul 2018 02:50:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DF47B28691; Tue, 24 Jul 2018 02:50:17 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 39BC328680 for ; Tue, 24 Jul 2018 02:50:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388464AbeGXDy1 (ORCPT ); Mon, 23 Jul 2018 23:54:27 -0400 Received: from sandeen.net ([63.231.237.45]:52804 "EHLO sandeen.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388367AbeGXDy1 (ORCPT ); Mon, 23 Jul 2018 23:54:27 -0400 Received: from Liberator-5.local (75-139-212-147.dhcp.knwc.wa.charter.com [75.139.212.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by sandeen.net (Postfix) with ESMTPSA id 07BF09C; Mon, 23 Jul 2018 21:49:20 -0500 (CDT) Subject: [PATCH 2/2 V3] xfs_repair: clear_dinode should simply clear, not check contents To: Eric Sandeen , linux-xfs References: From: Eric Sandeen Openpgp: preference=signencrypt Autocrypt: addr=sandeen@sandeen.net; prefer-encrypt=mutual; keydata= xsFNBE6x99QBEADMR+yNFBc1Y5avoUhzI/sdR9ANwznsNpiCtZlaO4pIWvqQJCjBzp96cpCs nQZV32nqJBYnDpBDITBqTa/EF+IrHx8gKq8TaSBLHUq2ju2gJJLfBoL7V3807PQcI18YzkF+ WL05ODFQ2cemDhx5uLghHEeOxuGj+1AI+kh/FCzMedHc6k87Yu2ZuaWF+Gh1W2ix6hikRJmQ vj5BEeAx7xKkyBhzdbNIbbjV/iGi9b26B/dNcyd5w2My2gxMtxaiP7q5b6GM2rsQklHP8FtW ZiYO7jsg/qIppR1C6Zr5jK1GQlMUIclYFeBbKggJ9mSwXJH7MIftilGQ8KDvNuV5AbkronGC sEEHj2khs7GfVv4pmUUHf1MRIvV0x3WJkpmhuZaYg8AdJlyGKgp+TQ7B+wCjNTdVqMI1vDk2 BS6Rg851ay7AypbCPx2w4d8jIkQEgNjACHVDU89PNKAjScK1aTnW+HNUqg9BliCvuX5g4z2j gJBs57loTWAGe2Ve3cMy3VoQ40Wt3yKK0Eno8jfgzgb48wyycINZgnseMRhxc2c8hd51tftK LKhPj4c7uqjnBjrgOVaVBupGUmvLiePlnW56zJZ51BR5igWnILeOJ1ZIcf7KsaHyE6B1mG+X dmYtjDhjf3NAcoBWJuj8euxMB6TcQN2MrSXy5wSKaw40evooGwARAQABzSVFcmljIFIuIFNh bmRlZW4gPHNhbmRlZW5Ac2FuZGVlbi5uZXQ+wsF7BBMBAgAlAhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgAUCUzMzbAIZAQAKCRAgrhaS4T3e4Fr7D/wO+fenqVvHjq21SCjDCrt8HdVj aJ28B1SqSU2toxyg5I160GllAxEHpLFGdbFAhQfBtnmlY9eMjwmJb0sCIrkrB6XNPSPA/B2B UPISh0z2odJv35/euJF71qIFgWzp2czJHkHWwVZaZpMWWNvsLIroXoR+uA9c2V1hQFVAJZyk EE4xzfm1+oVtjIC12B9tTCuS00pY3AUy21yzNowT6SSk7HAzmtG/PJ/uSB5wEkwldB6jVs2A sjOg1wMwVvh/JHilsQg4HSmDfObmZj1d0RWlMWcUE7csRnCE0ZWBMp/ttTn+oosioGa09HAS 9jAnauznmYg43oQ5Akd8iQRxz5I58F/+JsdKvWiyrPDfYZtFS+UIgWD7x+mHBZ53Qjazszox gjwO9ehZpwUQxBm4I0lPDAKw3HJA+GwwiubTSlq5PS3P7QoCjaV8llH1bNFZMz2o8wPANiDx 5FHgpRVgwLHakoCU1Gc+LXHXBzDXt7Cj02WYHdFzMm2hXaslRdhNGowLo1SXZFXa41KGTlNe 4di53y9CK5ynV0z+YUa+5LR6RdHrHtgywdKnjeWdqhoVpsWIeORtwWGX8evNOiKJ7j0RsHha WrePTubr5nuYTDsQqgc2r4aBIOpeSRR2brlT/UE3wGgy9LY78L4EwPR0MzzecfE1Ws60iSqw Pu3vhb7h3c7BTQROsffUARAA0DrUifTrXQzqxO8aiQOC5p9Tz25Np/Tfpv1rofOwL8VPBMvJ X4P5l1V2yd70MZRUVgjmCydEyxLJ6G2YyHO2IZTEajUY0Up+b3ErOpLpZwhvgWatjifpj6bB SKuDXeThqFdkphF5kAmgfVAIkan5SxWK3+S0V2F/oxstIViBhMhDwI6XsRlnVBoLLYcEilxA 2FlRUS7MOZGmRJkRtdGD5koVZSM6xVZQSmfEBaYQ/WJBGJQdPy94nnlAVn3lH3+N7pXvNUuC GV+t4YUt3tLcRuIpYBCOWlc7bpgeCps5Xa0dIZgJ8Louu6OBJ5vVXjPxTlkFdT0S0/uerCG5 1u8p6sGRLnUeAUGkQfIUqGUjW2rHaXgWNvzOV6i3tf9YaiXKl3avFaNW1kKBs0T5M1cnlWZU Utl6k04lz5OjoNY9J/bGyV3DSlkblXRMK87iLYQSrcV6cFz9PRl4vW1LGff3xRQHngeN5fPx ze8X5NE3hb+SSwyMSEqJxhVTXJVfQWWW0dQxP7HNwqmOWYF/6m+1gK/Y2gY3jAQnsWTru4RV TZGnKwEPmOCpSUvsTRXsVHgsWJ70qd0yOSjWuiv4b8vmD3+QFgyvCBxPMdP3xsxN5etheLMO gRwWpLn6yNFq/xtgs+ECgG+gR78yXQyA7iCs5tFs2OrMqV5juSMGmn0kxJUAEQEAAcLBXwQY AQIACQUCTrH31AIbDAAKCRAgrhaS4T3e4BKwD/0ZOOmUNOZCSOLAMjZx3mtYtjYgfUNKi0ki YPveGoRWTqbis8UitPtNrG4XxgzLOijSdOEzQwkdOIp/QnZhGNssMejCnsluK0GQd+RkFVWN mcQT78hBeGcnEMAXZKq7bkIKzvc06GFmkMbX/gAl6DiNGv0UNAX+5FYh+ucCJZSyAp3sA+9/ LKjxnTedX0aygXA6rkpX0Y0FvN/9dfm47+LGq7WAqBOyYTU3E6/+Z72bZoG/cG7ANLxcPool LOrU43oqFnD8QwcN56y4VfFj3/jDF2MX3xu4v2OjglVjMEYHTCxP3mpxesGHuqOit/FR+mF0 MP9JGfj6x+bj/9JMBtCW1bY/aPeMdPGTJvXjGtOVYblGZrSjXRn5++Uuy36CvkcrjuziSDG+ JEexGxczWwN4mrOQWhMT5Jyb+18CO+CWxJfHaYXiLEW7dI1AynL4jjn4W0MSiXpWDUw+fsBO Pk6ah10C4+R1Jc7dyUsKksMfvvhRX1hTIXhth85H16706bneTayZBhlZ/hK18uqTX+s0onG/ m1F3vYvdlE4p2ts1mmixMF7KajN9/E5RQtiSArvKTbfsB6Two4MthIuLuf+M0mI4gPl9SPlf fWCYVPhaU9o83y1KFbD/+lh1pjP7bEu/YudBvz7F2Myjh4/9GUAijrCTNeDTDAgvIJDjXuLX pA== Message-ID: <6d3277e7-610e-4777-7563-7026b21e8ac6@sandeen.net> Date: Mon, 23 Jul 2018 19:50:13 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Today clear_inode performs 2 separate tasks - it clears a disk inode when the calling code detects an inconsistency, and it also validates free inodes to some extent by checking each field before zeroing it out. This leads to unnecessary checks in the former case where we just want to zap the inode, and duplicates some of the existing verifier checks in the latter case. Now that we're using xfs_dinode_verify to validate free inodes, there's no reason to repeat all the checks inside clear_inode_core. Drop them all and simply clear it when told to do so. Signed-off-by: Eric Sandeen Reviewed-by: Carlos Maiolino Reviewed-by: Darrick J. Wong --- -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/repair/dinode.c b/repair/dinode.c index c0db15a..5b9fd9a 100644 --- a/repair/dinode.c +++ b/repair/dinode.c @@ -117,137 +117,24 @@ _("would have cleared inode %" PRIu64 " attributes\n"), ino_num); return(1); } -static int +static void clear_dinode_core(struct xfs_mount *mp, xfs_dinode_t *dinoc, xfs_ino_t ino_num) { - int dirty = 0; - int i; - -#define __dirty_no_modify_ret(dirty) \ - ({ (dirty) = 1; if (no_modify) return 1; }) - - if (be16_to_cpu(dinoc->di_magic) != XFS_DINODE_MAGIC) { - __dirty_no_modify_ret(dirty); - dinoc->di_magic = cpu_to_be16(XFS_DINODE_MAGIC); - } - - if (!libxfs_dinode_good_version(mp, dinoc->di_version)) { - __dirty_no_modify_ret(dirty); - if (xfs_sb_version_hascrc(&mp->m_sb)) - dinoc->di_version = 3; - else - dinoc->di_version = 2; - } - - if (be16_to_cpu(dinoc->di_mode) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_mode = 0; - } - - if (be16_to_cpu(dinoc->di_flags) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_flags = 0; - } - - if (be32_to_cpu(dinoc->di_dmevmask) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_dmevmask = 0; - } - - if (dinoc->di_forkoff != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_forkoff = 0; - } - - if (dinoc->di_format != XFS_DINODE_FMT_EXTENTS) { - __dirty_no_modify_ret(dirty); - dinoc->di_format = XFS_DINODE_FMT_EXTENTS; - } - - if (dinoc->di_aformat != XFS_DINODE_FMT_EXTENTS) { - __dirty_no_modify_ret(dirty); - dinoc->di_aformat = XFS_DINODE_FMT_EXTENTS; - } - - if (be64_to_cpu(dinoc->di_size) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_size = 0; - } - - if (be64_to_cpu(dinoc->di_nblocks) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_nblocks = 0; - } - - if (be16_to_cpu(dinoc->di_onlink) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_onlink = 0; - } - - if (be32_to_cpu(dinoc->di_nextents) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_nextents = 0; - } - - if (be16_to_cpu(dinoc->di_anextents) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_anextents = 0; - } - - if (be32_to_cpu(dinoc->di_extsize) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_extsize = 0; - } - - if (dinoc->di_version > 1 && - be32_to_cpu(dinoc->di_nlink) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_nlink = 0; - } - + memset(dinoc, 0, sizeof(*dinoc)); + dinoc->di_magic = cpu_to_be16(XFS_DINODE_MAGIC); + if (xfs_sb_version_hascrc(&mp->m_sb)) + dinoc->di_version = 3; + else + dinoc->di_version = 2; + dinoc->di_gen = cpu_to_be32(random()); + dinoc->di_format = XFS_DINODE_FMT_EXTENTS; + dinoc->di_aformat = XFS_DINODE_FMT_EXTENTS; /* we are done for version 1/2 inodes */ if (dinoc->di_version < 3) - return dirty; - - if (be64_to_cpu(dinoc->di_ino) != ino_num) { - __dirty_no_modify_ret(dirty); - dinoc->di_ino = cpu_to_be64(ino_num); - } - - if (platform_uuid_compare(&dinoc->di_uuid, &mp->m_sb.sb_meta_uuid)) { - __dirty_no_modify_ret(dirty); - platform_uuid_copy(&dinoc->di_uuid, &mp->m_sb.sb_meta_uuid); - } - - for (i = 0; i < sizeof(dinoc->di_pad2)/sizeof(dinoc->di_pad2[0]); i++) { - if (dinoc->di_pad2[i] != 0) { - __dirty_no_modify_ret(dirty); - memset(dinoc->di_pad2, 0, sizeof(dinoc->di_pad2)); - break; - } - } - - if (be64_to_cpu(dinoc->di_flags2) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_flags2 = 0; - } - - if (be64_to_cpu(dinoc->di_lsn) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_lsn = 0; - } - - if (be64_to_cpu(dinoc->di_changecount) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_changecount = 0; - } - - if (be32_to_cpu(dinoc->di_cowextsize) != 0) { - __dirty_no_modify_ret(dirty); - dinoc->di_cowextsize = 0; - } - - return dirty; + return; + dinoc->di_ino = cpu_to_be64(ino_num); + platform_uuid_copy(&dinoc->di_uuid, &mp->m_sb.sb_meta_uuid); + return; } static int @@ -268,21 +155,15 @@ clear_dinode_unlinked(xfs_mount_t *mp, xfs_dinode_t *dino) * until after the agi unlinked lists are walked in phase 3. * returns > zero if the inode has been altered while being cleared */ -static int +static void clear_dinode(xfs_mount_t *mp, xfs_dinode_t *dino, xfs_ino_t ino_num) { - int dirty; - - dirty = clear_dinode_core(mp, dino, ino_num); - dirty += clear_dinode_unlinked(mp, dino); + clear_dinode_core(mp, dino, ino_num); + clear_dinode_unlinked(mp, dino); /* and clear the forks */ - - if (dirty && !no_modify) - memset(XFS_DFORK_DPTR(dino), 0, - XFS_LITINO(mp, dino->di_version)); - - return(dirty); + memset(XFS_DFORK_DPTR(dino), 0, XFS_LITINO(mp, dino->di_version)); + return; } @@ -2140,8 +2021,8 @@ process_inode_data_fork( if (err) { do_warn(_("bad data fork in inode %" PRIu64 "\n"), lino); if (!no_modify) { - *dirty += clear_dinode(mp, dino, lino); - ASSERT(*dirty > 0); + clear_dinode(mp, dino, lino); + *dirty += 1; } return 1; } @@ -2551,7 +2432,7 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"), } /* - * if not in verify mode, check to sii if the inode and imap + * if not in verify mode, check to see if the inode and imap * agree that the inode is free */ if (!verify_mode && di_mode == 0) { @@ -2568,8 +2449,9 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"), do_warn( _("free inode %" PRIu64 " contains errors, "), lino); if (!no_modify) { - *dirty += clear_dinode(mp, dino, lino); + clear_dinode(mp, dino, lino); do_warn(_("corrected\n")); + *dirty += 1; } else { do_warn(_("would correct\n")); } @@ -2586,7 +2468,8 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"), _("imap claims a free inode %" PRIu64 " is in use, "), lino); if (!no_modify) { do_warn(_("correcting imap and clearing inode\n")); - *dirty += clear_dinode(mp, dino, lino); + clear_dinode(mp, dino, lino); + *dirty += 1; retval = 1; } else do_warn(_("would correct imap and clear inode\n")); @@ -2804,8 +2687,10 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"), * we're going to find. check_dups is set to 1 only during * phase 4. Ugly. */ - if (check_dups && !no_modify) - *dirty += clear_dinode_unlinked(mp, dino); + if (check_dups && !no_modify) { + clear_dinode_unlinked(mp, dino); + *dirty += 1; + } /* set type and map type info */ @@ -3024,8 +2909,8 @@ _("Cannot have CoW extent size of zero on cowextsize inode %" PRIu64 ", "), clear_bad_out: if (!no_modify) { - *dirty += clear_dinode(mp, dino, lino); - ASSERT(*dirty > 0); + clear_dinode(mp, dino, lino); + *dirty += 1; } bad_out: *used = is_free;