diff mbox series

[03/12] btrfs: disk-io: allow btree_set_page_dirty() to do more sanity check on subpage metadata

Message ID 20210222063357.92930-4-wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: support read-write for subpage metadata | expand

Commit Message

Qu Wenruo Feb. 22, 2021, 6:33 a.m. UTC
For btree_set_page_dirty(), we should also check the extent buffer
sanity for subpage support.

Unlike the regular sector size case, since one page can contain multiple
extent buffers, we need to make sure there is at least one dirty extent
buffer in the page.

So this patch will iterate through the btrfs_subpage::dirty_bitmap
to get the extent buffers, and check if any dirty extent buffer in the page
range has EXTENT_BUFFER_DIRTY and proper refs.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c | 47 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)

Comments

Su Yue Feb. 22, 2021, 7:58 a.m. UTC | #1
On Mon 22 Feb 2021 at 14:33, Qu Wenruo <wqu@suse.com> wrote:

> For btree_set_page_dirty(), we should also check the extent 
> buffer
> sanity for subpage support.
>
> Unlike the regular sector size case, since one page can contain 
> multiple
> extent buffers, we need to make sure there is at least one dirty 
> extent
> buffer in the page.
>
> So this patch will iterate through the 
> btrfs_subpage::dirty_bitmap
> to get the extent buffers, and check if any dirty extent buffer 
> in the page
> range has EXTENT_BUFFER_DIRTY and proper refs.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/disk-io.c | 47 
>  ++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c2576c5fe62e..437e6b2163c7 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -42,6 +42,7 @@
>  #include "discard.h"
>  #include "space-info.h"
>  #include "zoned.h"
> +#include "subpage.h"
>
>  #define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN |\
>  				 BTRFS_HEADER_FLAG_RELOC |\
> @@ -992,14 +993,48 @@ static void btree_invalidatepage(struct 
> page *page, unsigned int offset,
>  static int btree_set_page_dirty(struct page *page)
>  {
>  #ifdef DEBUG
> +	struct btrfs_fs_info *fs_info = 
> btrfs_sb(page->mapping->host->i_sb);
> +	struct btrfs_subpage *subpage;
>  	struct extent_buffer *eb;
> +	int cur_bit;
>
cur_bit is not initialized.

> +	u64 page_start = page_offset(page);
> +
> +	if (fs_info->sectorsize == PAGE_SIZE) {
> +		BUG_ON(!PagePrivate(page));
> +		eb = (struct extent_buffer *)page->private;
> +		BUG_ON(!eb);
> +		BUG_ON(!test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> +		BUG_ON(!atomic_read(&eb->refs));
> +		btrfs_assert_tree_locked(eb);
> +		return __set_page_dirty_nobuffers(page);
> +	}
> +	ASSERT(PagePrivate(page) && page->private);
> +	subpage = (struct btrfs_subpage *)page->private;
> +
> +	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;
>
> -	BUG_ON(!PagePrivate(page));
> -	eb = (struct extent_buffer *)page->private;
> -	BUG_ON(!eb);
> -	BUG_ON(!test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
> -	BUG_ON(!atomic_read(&eb->refs));
> -	btrfs_assert_tree_locked(eb);
> +		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_locked(eb);
> +		free_extent_buffer(eb);
> +
> +		cur_bit += (fs_info->nodesize >> 
> fs_info->sectorsize_bits);
> +	}
>  #endif
>  	return __set_page_dirty_nobuffers(page);
>  }
David Sterba March 1, 2021, 4:29 p.m. UTC | #2
On Mon, Feb 22, 2021 at 03:58:00PM +0800, Su Yue wrote:
> 
> On Mon 22 Feb 2021 at 14:33, Qu Wenruo <wqu@suse.com> wrote:
> 
> > For btree_set_page_dirty(), we should also check the extent 
> > buffer
> > sanity for subpage support.
> >
> > Unlike the regular sector size case, since one page can contain 
> > multiple
> > extent buffers, we need to make sure there is at least one dirty 
> > extent
> > buffer in the page.
> >
> > So this patch will iterate through the 
> > btrfs_subpage::dirty_bitmap
> > to get the extent buffers, and check if any dirty extent buffer 
> > in the page
> > range has EXTENT_BUFFER_DIRTY and proper refs.
> >
> > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > ---
> >  fs/btrfs/disk-io.c | 47 
> >  ++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 41 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index c2576c5fe62e..437e6b2163c7 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -42,6 +42,7 @@
> >  #include "discard.h"
> >  #include "space-info.h"
> >  #include "zoned.h"
> > +#include "subpage.h"
> >
> >  #define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN |\
> >  				 BTRFS_HEADER_FLAG_RELOC |\
> > @@ -992,14 +993,48 @@ static void btree_invalidatepage(struct 
> > page *page, unsigned int offset,
> >  static int btree_set_page_dirty(struct page *page)
> >  {
> >  #ifdef DEBUG
> > +	struct btrfs_fs_info *fs_info = 
> > btrfs_sb(page->mapping->host->i_sb);
> > +	struct btrfs_subpage *subpage;
> >  	struct extent_buffer *eb;
> > +	int cur_bit;
> >
> cur_bit is not initialized.

Indeed and it's strange that gcc does not warn about that, either by
default or with W=3.
diff mbox series

Patch

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c2576c5fe62e..437e6b2163c7 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -42,6 +42,7 @@ 
 #include "discard.h"
 #include "space-info.h"
 #include "zoned.h"
+#include "subpage.h"
 
 #define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN |\
 				 BTRFS_HEADER_FLAG_RELOC |\
@@ -992,14 +993,48 @@  static void btree_invalidatepage(struct page *page, unsigned int offset,
 static int btree_set_page_dirty(struct page *page)
 {
 #ifdef DEBUG
+	struct btrfs_fs_info *fs_info = btrfs_sb(page->mapping->host->i_sb);
+	struct btrfs_subpage *subpage;
 	struct extent_buffer *eb;
+	int cur_bit;
+	u64 page_start = page_offset(page);
+
+	if (fs_info->sectorsize == PAGE_SIZE) {
+		BUG_ON(!PagePrivate(page));
+		eb = (struct extent_buffer *)page->private;
+		BUG_ON(!eb);
+		BUG_ON(!test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
+		BUG_ON(!atomic_read(&eb->refs));
+		btrfs_assert_tree_locked(eb);
+		return __set_page_dirty_nobuffers(page);
+	}
+	ASSERT(PagePrivate(page) && page->private);
+	subpage = (struct btrfs_subpage *)page->private;
+
+	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;
 
-	BUG_ON(!PagePrivate(page));
-	eb = (struct extent_buffer *)page->private;
-	BUG_ON(!eb);
-	BUG_ON(!test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
-	BUG_ON(!atomic_read(&eb->refs));
-	btrfs_assert_tree_locked(eb);
+		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_locked(eb);
+		free_extent_buffer(eb);
+
+		cur_bit += (fs_info->nodesize >> fs_info->sectorsize_bits);
+	}
 #endif
 	return __set_page_dirty_nobuffers(page);
 }