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 X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7C0E2C4361B for ; Fri, 18 Dec 2020 15:05:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3577823AFD for ; Fri, 18 Dec 2020 15:05:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728810AbgLRPFo (ORCPT ); Fri, 18 Dec 2020 10:05:44 -0500 Received: from mx2.suse.de ([195.135.220.15]:52990 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727734AbgLRPFn (ORCPT ); Fri, 18 Dec 2020 10:05:43 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 7DE64AD09; Fri, 18 Dec 2020 15:05:02 +0000 (UTC) Received: by ds.suse.cz (Postfix, from userid 10065) id 0F5DBDA806; Fri, 18 Dec 2020 16:03:21 +0100 (CET) Date: Fri, 18 Dec 2020 16:03:21 +0100 From: David Sterba To: Nikolay Borisov Cc: dsterba@suse.cz, linux-btrfs@vger.kernel.org Subject: Re: [PATCH 3/6] btrfs: Remove useless ASSERTS Message-ID: <20201218150321.GY6430@twin.jikos.cz> Reply-To: dsterba@suse.cz Mail-Followup-To: dsterba@suse.cz, Nikolay Borisov , linux-btrfs@vger.kernel.org References: <20201207153237.1073887-1-nborisov@suse.com> <20201207153237.1073887-4-nborisov@suse.com> <20201215165857.GX6430@twin.jikos.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Tue, Dec 15, 2020 at 07:48:18PM +0200, Nikolay Borisov wrote: > > > On 15.12.20 г. 18:58 ч., David Sterba wrote: > > On Mon, Dec 07, 2020 at 05:32:34PM +0200, Nikolay Borisov wrote: > >> The invariants the asserts are checking are already verified by the > >> tree checker, just remove them. > > > > I haven't found where exactly does tree-checker verify the invariant and > > also think that we can safely leave the asserts there. Even if it's for > > a normally impossible case, assertions usually catch bugs after changing > > some other code. > > > > 2 if (unlikely((key->objectid < BTRFS_ > 1 key->objectid > BTRFS_ #define BTRFS_ROOT_TREE_DIR_OBJECTID 6ULL > 402 key->objectid != BTRFS_ROOT_TREE_DIR_OBJECTID && > 1 key->objectid != BTRFS_FREE_INO_OBJECTID)) { > > > in check_inode_key. We verify that for every inode its objectid is > within range, transitively Ah so it's only indirectly implied. > this assures highest_objectid is also > within range. But If you want to leave it - I'm fine with it. Tree checker verifies that any inode that is read has the object id within the bounds, that's fine. The highest free objectid is obtained by doing reverse search, without reading (and checking) any existing inode. btrfs_init_root_free_objectid checks only object ids in the tree, not necessarily inodes (though technically we use the objectids only for inode-like items). Things can be improved by doing proper checks inside btrfs_init_root_free_objectid and drop the asserts, I can imagine a crafted image that would trigger the asserts so we'd better handle that.