[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240109075642.43a10c35@canb.auug.org.au>
Date: Tue, 9 Jan 2024 07:56:42 +1100
From: Stephen Rothwell <sfr@...b.auug.org.au>
To: David Sterba <dsterba@...e.cz>
Cc: Christian Brauner <brauner@...nel.org>, David Sterba <dsterba@...e.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Linux Next
Mailing List <linux-next@...r.kernel.org>, "Matthew Wilcox (Oracle)"
<willy@...radead.org>, Qu Wenruo <wqu@...e.com>
Subject: Re: linux-next: manual merge of the vfs-brauner tree with the btrfs
tree
Hi all,
On Thu, 7 Dec 2023 10:32:13 +1100 Stephen Rothwell <sfr@...b.auug.org.au> 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
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists