linux-kernel - Re: [PATCH] btrfs: remove btree_dirty

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fc88a524-7b24-4f2c-91dd-a9e1d7ddd3c0@gmx.com>
Date:   Fri, 1 Sep 2023 17:40:21 +0800
From:   Qu Wenruo <quwenruo.btrfs@....com>
To:     Yunhui Cui <cuiyunhui@...edance.com>, clm@...com,
        josef@...icpanda.com, dsterba@...e.com,
        linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: remove btree_dirty_folio() inside DEBUG



On 2023/9/1 11:18, Yunhui Cui wrote:
> When the DEBUG compilation option is enabled (KCFLAGS=-DDEBUG),
> the following compilation error will occur:
> fs/btrfs/disk-io.c: In function ‘btree_dirty_folio’:
> fs/btrfs/disk-io.c:538:16: error: ‘struct btrfs_subpage’ has no member named ‘dirty_bitmap’
>    ASSERT(subpage->dirty_bitmap);
>                  ^~
> fs/btrfs/messages.h:181:29: note: in definition of macro ‘ASSERT’
>   #define ASSERT(expr) (void)(expr)
>                               ^~~~
> fs/btrfs/disk-io.c:539:19: error: ‘BTRFS_SUBPAGE_BITMAP_SIZE’ undeclared (first use in this function); did you mean ‘BTRFS_FREE_SPACE_BITMAP_SIZE’?
>    while (cur_bit < BTRFS_SUBPAGE_BITMAP_SIZE) {
>                     ^~~~~~~~~~~~~~~~~~~~~~~~~
>                     BTRFS_FREE_SPACE_BITMAP_SIZE
> fs/btrfs/disk-io.c:539:19: note: each undeclared identifier is reported only once for each function it appears in
> fs/btrfs/disk-io.c:545:22: error: ‘struct btrfs_subpage’ has no member named ‘dirty_bitmap’
>     if (!(tmp & subpage->dirty_bitmap)) {
>                        ^~
>
> Signed-off-by: Yunhui Cui <cuiyunhui@...edance.com>

There is already a proper fix for it other than deleting it completely:

https://lore.kernel.org/linux-btrfs/14ea9aa0a4d5ac8f2c817978e9fff6ded8777ef9.1692683147.git.wqu@suse.com

Thanks,
Qu
> ---
>   fs/btrfs/disk-io.c | 52 +---------------------------------------------
>   1 file changed, 1 insertion(+), 51 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 0a96ea8c1d3a..f0252c70233a 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -515,62 +515,12 @@ static void btree_invalidate_folio(struct folio *folio, size_t offset,
>   	}
>   }
>
> -#ifdef DEBUG
> -static bool btree_dirty_folio(struct address_space *mapping,
> -		struct folio *folio)
> -{
> -	struct btrfs_fs_info *fs_info = btrfs_sb(mapping->host->i_sb);
> -	struct btrfs_subpage *subpage;
> -	struct extent_buffer *eb;
> -	int cur_bit = 0;
> -	u64 page_start = folio_pos(folio);
> -
> -	if (fs_info->sectorsize == PAGE_SIZE) {
> -		eb = folio_get_private(folio);
> -		BUG_ON(!eb);
> -		BUG_ON(!test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> -		BUG_ON(!atomic_read(&eb->refs));
> -		btrfs_assert_tree_write_locked(eb);
> -		return filemap_dirty_folio(mapping, folio);
> -	}
> -	subpage = folio_get_private(folio);
> -
> -	ASSERT(subpage->dirty_bitmap);
> -	while (cur_bit < BTRFS_SUBPAGE_BITMAP_SIZE) {
> -		unsigned long flags;
> -		u64 cur;
> -		u16 tmp = (1 << cur_bit);
> -
> -		spin_lock_irqsave(&subpage->lock, flags);
> -		if (!(tmp & subpage->dirty_bitmap)) {
> -			spin_unlock_irqrestore(&subpage->lock, flags);
> -			cur_bit++;
> -			continue;
> -		}
> -		spin_unlock_irqrestore(&subpage->lock, flags);
> -		cur = page_start + cur_bit * fs_info->sectorsize;
> -
> -		eb = find_extent_buffer(fs_info, cur);
> -		ASSERT(eb);
> -		ASSERT(test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> -		ASSERT(atomic_read(&eb->refs));
> -		btrfs_assert_tree_write_locked(eb);
> -		free_extent_buffer(eb);
> -
> -		cur_bit += (fs_info->nodesize >> fs_info->sectorsize_bits);
> -	}
> -	return filemap_dirty_folio(mapping, folio);
> -}
> -#else
> -#define btree_dirty_folio filemap_dirty_folio
> -#endif
> -
>   static const struct address_space_operations btree_aops = {
>   	.writepages	= btree_writepages,
>   	.release_folio	= btree_release_folio,
>   	.invalidate_folio = btree_invalidate_folio,
>   	.migrate_folio	= btree_migrate_folio,
> -	.dirty_folio	= btree_dirty_folio,
> +	.dirty_folio	= filemap_dirty_folio,
>   };
>
>   struct extent_buffer *btrfs_find_create_tree_block(