diff mbox series

[v4,07/18] btrfs: attach private to dummy extent buffer pages

Message ID 20210116071533.105780-8-wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: add read-only support for subpage sector size | expand

Commit Message

Qu Wenruo Jan. 16, 2021, 7:15 a.m. UTC
Even for regular btrfs, there are locations where we allocate dummy
extent buffers for temporary usage.

Like tree_mod_log_rewind() and get_old_root().

Those dummy extent buffers will be handled by the same eb accessors, and
if they don't have page::private subpage eb accessors can fail.

To address such problems, make __alloc_dummy_extent_buffer() to attach
page private for dummy extent buffers too.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent_io.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Josef Bacik Jan. 20, 2021, 2:48 p.m. UTC | #1
On 1/16/21 2:15 AM, Qu Wenruo wrote:
> Even for regular btrfs, there are locations where we allocate dummy
> extent buffers for temporary usage.
> 
> Like tree_mod_log_rewind() and get_old_root().
> 
> Those dummy extent buffers will be handled by the same eb accessors, and
> if they don't have page::private subpage eb accessors can fail.
> 
> To address such problems, make __alloc_dummy_extent_buffer() to attach
> page private for dummy extent buffers too.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

We already know these eb's are fake because they have UNMAPPED set, just adjust 
your subpage helpers to be no-op if UNMAPPED is set.  Thanks,

Josef
Qu Wenruo Jan. 21, 2021, 12:47 a.m. UTC | #2
On 2021/1/20 下午10:48, Josef Bacik wrote:
> On 1/16/21 2:15 AM, Qu Wenruo wrote:
>> Even for regular btrfs, there are locations where we allocate dummy
>> extent buffers for temporary usage.
>>
>> Like tree_mod_log_rewind() and get_old_root().
>>
>> Those dummy extent buffers will be handled by the same eb accessors, and
>> if they don't have page::private subpage eb accessors can fail.
>>
>> To address such problems, make __alloc_dummy_extent_buffer() to attach
>> page private for dummy extent buffers too.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> We already know these eb's are fake because they have UNMAPPED set, just
> adjust your subpage helpers to be no-op if UNMAPPED is set.  Thanks,

But then the helper behavior would be a mess.

Some accessors, like read/write_extent_buffer() will still do subpage
specific offset calcuation, even it has UNMAPPED bit.

Thus I still prefer to do the same operations, reducing the branches in
accessors.

Thanks,
Qu
>
> Josef
diff mbox series

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index fb800f237099..7f94f00936d7 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -5204,9 +5204,14 @@  struct extent_buffer *__alloc_dummy_extent_buffer(struct btrfs_fs_info *fs_info,
 
 	num_pages = num_extent_pages(eb);
 	for (i = 0; i < num_pages; i++) {
+		int ret;
+
 		eb->pages[i] = alloc_page(GFP_NOFS);
 		if (!eb->pages[i])
 			goto err;
+		ret = attach_extent_buffer_page(eb, eb->pages[i], NULL);
+		if (ret < 0)
+			goto err;
 	}
 	set_extent_buffer_uptodate(eb);
 	btrfs_set_header_nritems(eb, 0);
@@ -5214,8 +5219,10 @@  struct extent_buffer *__alloc_dummy_extent_buffer(struct btrfs_fs_info *fs_info,
 
 	return eb;
 err:
-	for (; i > 0; i--)
+	for (; i > 0; i--) {
+		detach_extent_buffer_page(eb, eb->pages[i - 1]);
 		__free_page(eb->pages[i - 1]);
+	}
 	__free_extent_buffer(eb);
 	return NULL;
 }