Re: linux-next: manual merge of the vfs-brauner tree with the btrfs tree

[Stephen Rothwell]

Hi all,

On Thu, 7 Dec 2023 10:32:13 +1100 Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
> 
> Today's linux-next merge of the vfs-brauner tree got a conflict in:
> 
>   fs/btrfs/extent_io.c
> 
> between commits:
> 
>   16aee93de711 ("btrfs: migrate to use folio private instead of page private")
>   042eab832c43 ("btrfs: refactor alloc_extent_buffer() to allocate-then-attach method")
> 
> from the btrfs tree and commit:
> 
>   600f111ef51d ("fs: Rename mapping private members")
> 
> from the vfs-brauner tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc fs/btrfs/extent_io.c
> index 5cae7884e8d9,3431a53bf3fd..000000000000
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@@ -881,13 -870,13 +881,13 @@@ static int attach_extent_buffer_page(st
>   	 * will not race with any other ebs.
>   	 */
>   	if (page->mapping)
> - 		lockdep_assert_held(&page->mapping->private_lock);
> + 		lockdep_assert_held(&page->mapping->i_private_lock);
>   
>   	if (fs_info->nodesize >= PAGE_SIZE) {
>  -		if (!PagePrivate(page))
>  -			attach_page_private(page, eb);
>  +		if (!folio_test_private(folio))
>  +			folio_attach_private(folio, eb);
>   		else
>  -			WARN_ON(page->private != (unsigned long)eb);
>  +			WARN_ON(folio_get_private(folio) != eb);
>   		return 0;
>   	}
>   
> @@@ -1750,9 -1736,9 +1750,9 @@@ static int submit_eb_subpage(struct pag
>   		 * Take private lock to ensure the subpage won't be detached
>   		 * in the meantime.
>   		 */
> - 		spin_lock(&page->mapping->private_lock);
> + 		spin_lock(&page->mapping->i_private_lock);
>  -		if (!PagePrivate(page)) {
>  +		if (!folio_test_private(folio)) {
> - 			spin_unlock(&page->mapping->private_lock);
> + 			spin_unlock(&page->mapping->i_private_lock);
>   			break;
>   		}
>   		spin_lock_irqsave(&subpage->lock, flags);
> @@@ -1826,9 -1811,9 +1826,9 @@@ static int submit_eb_page(struct page *
>   	if (btrfs_sb(page->mapping->host->i_sb)->nodesize < PAGE_SIZE)
>   		return submit_eb_subpage(page, wbc);
>   
> - 	spin_lock(&mapping->private_lock);
> + 	spin_lock(&mapping->i_private_lock);
>  -	if (!PagePrivate(page)) {
>  +	if (!folio_test_private(folio)) {
> - 		spin_unlock(&mapping->private_lock);
> + 		spin_unlock(&mapping->i_private_lock);
>   		return 0;
>   	}
>   
> @@@ -3070,13 -3054,12 +3070,13 @@@ static int extent_buffer_under_io(cons
>   
>   static bool page_range_has_eb(struct btrfs_fs_info *fs_info, struct page *page)
>   {
>  +	struct folio *folio = page_folio(page);
>   	struct btrfs_subpage *subpage;
>   
> - 	lockdep_assert_held(&page->mapping->private_lock);
> + 	lockdep_assert_held(&page->mapping->i_private_lock);
>   
>  -	if (PagePrivate(page)) {
>  -		subpage = (struct btrfs_subpage *)page->private;
>  +	if (folio_test_private(folio)) {
>  +		subpage = folio_get_private(folio);
>   		if (atomic_read(&subpage->eb_refs))
>   			return true;
>   		/*
> @@@ -3093,18 -3076,17 +3093,18 @@@ static void detach_extent_buffer_page(s
>   {
>   	struct btrfs_fs_info *fs_info = eb->fs_info;
>   	const bool mapped = !test_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags);
>  +	struct folio *folio = page_folio(page);
>   
>   	/*
>  -	 * For mapped eb, we're going to change the page private, which should
>  +	 * For mapped eb, we're going to change the folio private, which should
> - 	 * be done under the private_lock.
> + 	 * be done under the i_private_lock.
>   	 */
>   	if (mapped)
> - 		spin_lock(&page->mapping->private_lock);
> + 		spin_lock(&page->mapping->i_private_lock);
>   
>  -	if (!PagePrivate(page)) {
>  +	if (!folio_test_private(folio)) {
>   		if (mapped)
> - 			spin_unlock(&page->mapping->private_lock);
> + 			spin_unlock(&page->mapping->i_private_lock);
>   		return;
>   	}
>   
> @@@ -3120,11 -3103,14 +3120,11 @@@
>   			BUG_ON(test_bit(EXTENT_BUFFER_DIRTY, &eb->bflags));
>   			BUG_ON(PageDirty(page));
>   			BUG_ON(PageWriteback(page));
>  -			/*
>  -			 * We need to make sure we haven't be attached
>  -			 * to a new eb.
>  -			 */
>  -			detach_page_private(page);
>  +			/* We need to make sure we haven't be attached to a new eb. */
>  +			folio_detach_private(folio);
>   		}
>   		if (mapped)
> - 			spin_unlock(&page->mapping->private_lock);
> + 			spin_unlock(&page->mapping->i_private_lock);
>   		return;
>   	}
>   
> @@@ -3588,8 -3513,8 +3588,8 @@@ struct extent_buffer *alloc_extent_buff
>   	num_pages = num_extent_pages(eb);
>   
>   	/*
>  -	 * Preallocate page->private for subpage case, so that we won't
>  +	 * Preallocate folio private for subpage case, so that we won't
> - 	 * allocate memory with private_lock nor page lock hold.
> + 	 * allocate memory with i_private_lock nor page lock hold.
>   	 *
>   	 * The memory will be freed by attach_extent_buffer_page() or freed
>   	 * manually if we exit earlier.
> @@@ -3602,31 -3527,24 +3602,31 @@@
>   		}
>   	}
>   
>  -	for (i = 0; i < num_pages; i++, index++) {
>  -		p = find_or_create_page(mapping, index, GFP_NOFS|__GFP_NOFAIL);
>  -		if (!p) {
>  -			exists = ERR_PTR(-ENOMEM);
>  -			btrfs_free_subpage(prealloc);
>  -			goto free_eb;
>  -		}
>  +	/* Allocate all pages first. */
>  +	ret = btrfs_alloc_page_array(num_pages, eb->pages, __GFP_NOFAIL);
>  +	if (ret < 0) {
>  +		btrfs_free_subpage(prealloc);
>  +		goto out;
>  +	}
>   
>  -		spin_lock(&mapping->i_private_lock);
>  -		exists = grab_extent_buffer(fs_info, p);
>  -		if (exists) {
>  -			spin_unlock(&mapping->i_private_lock);
>  -			unlock_page(p);
>  -			put_page(p);
>  -			mark_extent_buffer_accessed(exists, p);
>  -			btrfs_free_subpage(prealloc);
>  -			goto free_eb;
>  +	/* Attach all pages to the filemap. */
>  +	for (int i = 0; i < num_pages; i++) {
>  +		struct page *p;
>  +
>  +		ret = attach_eb_page_to_filemap(eb, i, &existing_eb);
>  +		if (ret > 0) {
>  +			ASSERT(existing_eb);
>  +			goto out;
>   		}
>  +		attached++;
>  +
>  +		/*
>  +		 * Only after attach_eb_page_to_filemap(), eb->pages[] is
>  +		 * reliable, as we may choose to reuse the existing page cache
>  +		 * and free the allocated page.
>  +		 */
>  +		p = eb->pages[i];
> - 		spin_lock(&mapping->private_lock);
> ++		spin_lock(&mapping->i_private_lock);
>   		/* Should not fail, as we have preallocated the memory */
>   		ret = attach_extent_buffer_page(eb, p, prealloc);
>   		ASSERT(!ret);
> @@@ -3640,17 -3558,10 +3640,17 @@@
>   		 * Thus needs no special handling in error path.
>   		 */
>   		btrfs_page_inc_eb_refs(fs_info, p);
> - 		spin_unlock(&mapping->private_lock);
> + 		spin_unlock(&mapping->i_private_lock);
>   
>   		WARN_ON(btrfs_page_test_dirty(fs_info, p, eb->start, eb->len));
>  -		eb->pages[i] = p;
>  +
>  +		/*
>  +		 * Check if the current page is physically contiguous with previous eb
>  +		 * page.
>  +		 */
>  +		if (i && eb->pages[i - 1] + 1 != p)
>  +			page_contig = false;
>  +
>   		if (!btrfs_page_test_uptodate(fs_info, p, eb->start, eb->len))
>   			uptodate = 0;
>   
> @@@ -4713,11 -4560,11 +4713,11 @@@ static int try_release_subpage_extent_b
>   		release_extent_buffer(eb);
>   	}
>   	/*
>  -	 * Finally to check if we have cleared page private, as if we have
>  -	 * released all ebs in the page, the page private should be cleared now.
>  +	 * Finally to check if we have cleared folio private, as if we have
>  +	 * released all ebs in the page, the folio private should be cleared now.
>   	 */
> - 	spin_lock(&page->mapping->private_lock);
> + 	spin_lock(&page->mapping->i_private_lock);
>  -	if (!PagePrivate(page))
>  +	if (!folio_test_private(page_folio(page)))
>   		ret = 1;
>   	else
>   		ret = 0;
> @@@ -4735,12 -4581,12 +4735,12 @@@ int try_release_extent_buffer(struct pa
>   		return try_release_subpage_extent_buffer(page);
>   
>   	/*
>  -	 * We need to make sure nobody is changing page->private, as we rely on
>  -	 * page->private as the pointer to extent buffer.
>  +	 * We need to make sure nobody is changing folio private, as we rely on
>  +	 * folio private as the pointer to extent buffer.
>   	 */
> - 	spin_lock(&page->mapping->private_lock);
> + 	spin_lock(&page->mapping->i_private_lock);
>  -	if (!PagePrivate(page)) {
>  +	if (!folio_test_private(folio)) {
> - 		spin_unlock(&page->mapping->private_lock);
> + 		spin_unlock(&page->mapping->i_private_lock);
>   		return 1;
>   	}
>   

This is now a conflict between the btrfs tree and Linus' tree.

-- 
Cheers,
Stephen Rothwell

Attachment: pgpwmBf1GtMgn.pgp
Description: OpenPGP digital signature