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.5 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 A6E8FC433EC for ; Mon, 27 Jul 2020 15:17:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8FEFE2070B for ; Mon, 27 Jul 2020 15:17:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729200AbgG0PRy (ORCPT ); Mon, 27 Jul 2020 11:17:54 -0400 Received: from mx2.suse.de ([195.135.220.15]:48554 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728824AbgG0PRq (ORCPT ); Mon, 27 Jul 2020 11:17:46 -0400 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 07B93AC97; Mon, 27 Jul 2020 15:17:56 +0000 (UTC) Received: by ds.suse.cz (Postfix, from userid 10065) id 5AD42DA701; Mon, 27 Jul 2020 17:17:17 +0200 (CEST) Date: Mon, 27 Jul 2020 17:17:17 +0200 From: David Sterba To: Josef Bacik Cc: dsterba@suse.cz, linux-btrfs@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH 1/2] btrfs: free fs roots on failed mount Message-ID: <20200727151717.GO3703@twin.jikos.cz> Reply-To: dsterba@suse.cz Mail-Followup-To: dsterba@suse.cz, Josef Bacik , linux-btrfs@vger.kernel.org, kernel-team@fb.com References: <20200722160722.8641-1-josef@toxicpanda.com> <20200727141947.GN3703@twin.jikos.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Mon, Jul 27, 2020 at 10:33:32AM -0400, Josef Bacik wrote: > >> @@ -3441,6 +3440,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device > >> btrfs_put_block_group_cache(fs_info); > >> > >> fail_tree_roots: > >> + btrfs_free_fs_roots(fs_info); > >> free_root_pointers(fs_info, true); > > > > The data reloc tree is freed inside free_root_pointers, that it's also > > in the radix tree is for convenience so I'd rather fix it inside > > free_root_pointers and not reorder btrfs_free_fs_roots. > > We can't do that, because free_root_pointers() is called to drop the extent > buffers when we're looping through the backup roots. btrfs_free_fs_roots() is > the correct thing to do here, it goes through anything that ended up in the > radix tree. Thanks, I see, free_root_pointers is used elsewhere. I'm concerned about the reordeing because there are several functions that are now called after the roots are freed. (before) btrfs_free_fs_roots(fs_info); kthread_stop(fs_info->cleaner_kthread); filemap_write_and_wait(fs_info->btree_inode->i_mapping); btrfs_sysfs_remove_mounted(fs_info); btrfs_sysfs_remove_fsid(fs_info->fs_devices); btrfs_put_block_group_cache(fs_info); (after) btrfs_free_fs_roots(fs_info); If something called by btrfs_free_fs_roots depends on structures removed/cleaned by the other functions, eg. btrfs_put_block_group_cache ti could be a problem. I haven't spotted anything so far, the functions are called after failure still during mount, this is easier than shutdown sequence after a full mount.