linux-next: manual merge of the folio tree with the ext4 tree

linux-next: manual merge of the folio tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3378 bytes --]

Hi all,

Today's linux-next merge of the folio tree got a conflict in:

  fs/ext4/inode.c

between commit:

  2bb8dd401a4f ("ext4: warn when dirtying page w/o buffers in data=journal mode")

from the ext4 tree and commit:

  821405cf3ebb ("fs: Convert trivial uses of __set_page_dirty_nobuffers to filemap_dirty_folio")

from the folio tree.

I didn't know how to complete this fix up ans so just commented out the
new WARN_ON().

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/ext4/inode.c
index 3d0ca48d20c8,436efd31cc27..000000000000
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@@ -3573,31 -3541,30 +3573,32 @@@ const struct iomap_ops ext4_iomap_repor
  };
  
  /*
-  * Whenever the page is being dirtied, corresponding buffers should already be
 - * Folios can be marked dirty completely asynchronously from ext4's
 - * journalling activity.  By filemap_sync_pte(), try_to_unmap_one(), etc.
 - * We cannot do much here because ->dirty_folio may be called with the
 - * page table lock held.  The folio is not necessarily locked.
++ * Whenever the folio is being dirtied, corresponding buffers should already be
 + * attached to the transaction (we take care of this in ext4_page_mkwrite() and
 + * ext4_write_begin()). However we cannot move buffers to dirty transaction
-  * lists here because ->set_page_dirty is called under VFS locks and the page
++ * lists here because ->dirty_folio is called under VFS locks and the folio
 + * is not necessarily locked.
   *
-  * We cannot just dirty the page and leave attached buffers clean, because the
+  * We cannot just dirty the folio and leave attached buffers clean, because the
   * buffers' dirty state is "definitive".  We cannot just set the buffers dirty
   * or jbddirty because all the journalling code will explode.
   *
-  * So what we do is to mark the page "pending dirty" and next time writepage
+  * So what we do is to mark the folio "pending dirty" and next time writepage
   * is called, propagate that into the buffers appropriately.
   */
- static int ext4_journalled_set_page_dirty(struct page *page)
+ static bool ext4_journalled_dirty_folio(struct address_space *mapping,
+ 		struct folio *folio)
  {
- 	WARN_ON_ONCE(!page_has_buffers(page));
- 	SetPageChecked(page);
- 	return __set_page_dirty_nobuffers(page);
++/*	WARN_ON_ONCE(!page_has_buffers(page)); */
+ 	folio_set_checked(folio);
+ 	return filemap_dirty_folio(mapping, folio);
  }
  
- static int ext4_set_page_dirty(struct page *page)
+ static bool ext4_dirty_folio(struct address_space *mapping, struct folio *folio)
  {
- 	WARN_ON_ONCE(!PageLocked(page) && !PageDirty(page));
- 	WARN_ON_ONCE(!page_has_buffers(page));
- 	return __set_page_dirty_buffers(page);
+ 	WARN_ON_ONCE(!folio_test_locked(folio) && !folio_test_dirty(folio));
+ 	WARN_ON_ONCE(!folio_buffers(folio));
+ 	return block_dirty_folio(mapping, folio);
  }
  
  static int ext4_iomap_swap_activate(struct swap_info_struct *sis,

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the folio tree with the ext4 tree

[Jan Kara]

On Tue 15-03-22 20:40:07, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the folio tree got a conflict in:
> 
>   fs/ext4/inode.c
> 
> between commit:
> 
>   2bb8dd401a4f ("ext4: warn when dirtying page w/o buffers in data=journal mode")
> 
> from the ext4 tree and commit:
> 
>   821405cf3ebb ("fs: Convert trivial uses of __set_page_dirty_nobuffers to filemap_dirty_folio")
> 
> from the folio tree.
> 
> I didn't know how to complete this fix up ans so just commented out the
> new WARN_ON().

Thanks for the notice Stephen! The resolution should be like:

- 	WARN_ON_ONCE(!page_has_buffers(page));
+ 	WARN_ON_ONCE(!folio_buffers(folio));

How are we going to handle this conflict Ted? Just tell Linus about the
conflict and resolution?

								Honza

> 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/ext4/inode.c
> index 3d0ca48d20c8,436efd31cc27..000000000000
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@@ -3573,31 -3541,30 +3573,32 @@@ const struct iomap_ops ext4_iomap_repor
>   };
>   
>   /*
> -  * Whenever the page is being dirtied, corresponding buffers should already be
>  - * Folios can be marked dirty completely asynchronously from ext4's
>  - * journalling activity.  By filemap_sync_pte(), try_to_unmap_one(), etc.
>  - * We cannot do much here because ->dirty_folio may be called with the
>  - * page table lock held.  The folio is not necessarily locked.
> ++ * Whenever the folio is being dirtied, corresponding buffers should already be
>  + * attached to the transaction (we take care of this in ext4_page_mkwrite() and
>  + * ext4_write_begin()). However we cannot move buffers to dirty transaction
> -  * lists here because ->set_page_dirty is called under VFS locks and the page
> ++ * lists here because ->dirty_folio is called under VFS locks and the folio
>  + * is not necessarily locked.
>    *
> -  * We cannot just dirty the page and leave attached buffers clean, because the
> +  * We cannot just dirty the folio and leave attached buffers clean, because the
>    * buffers' dirty state is "definitive".  We cannot just set the buffers dirty
>    * or jbddirty because all the journalling code will explode.
>    *
> -  * So what we do is to mark the page "pending dirty" and next time writepage
> +  * So what we do is to mark the folio "pending dirty" and next time writepage
>    * is called, propagate that into the buffers appropriately.
>    */
> - static int ext4_journalled_set_page_dirty(struct page *page)
> + static bool ext4_journalled_dirty_folio(struct address_space *mapping,
> + 		struct folio *folio)
>   {
> - 	WARN_ON_ONCE(!page_has_buffers(page));
> - 	SetPageChecked(page);
> - 	return __set_page_dirty_nobuffers(page);
> ++/*	WARN_ON_ONCE(!page_has_buffers(page)); */
> + 	folio_set_checked(folio);
> + 	return filemap_dirty_folio(mapping, folio);
>   }
>   
> - static int ext4_set_page_dirty(struct page *page)
> + static bool ext4_dirty_folio(struct address_space *mapping, struct folio *folio)
>   {
> - 	WARN_ON_ONCE(!PageLocked(page) && !PageDirty(page));
> - 	WARN_ON_ONCE(!page_has_buffers(page));
> - 	return __set_page_dirty_buffers(page);
> + 	WARN_ON_ONCE(!folio_test_locked(folio) && !folio_test_dirty(folio));
> + 	WARN_ON_ONCE(!folio_buffers(folio));
> + 	return block_dirty_folio(mapping, folio);
>   }
>   
>   static int ext4_iomap_swap_activate(struct swap_info_struct *sis,


-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: linux-next: manual merge of the folio tree with the ext4 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 337 bytes --]

Hi Jan,

On Tue, 15 Mar 2022 12:25:12 +0100 Jan Kara <jack@suse.cz> wrote:
>
> Thanks for the notice Stephen! The resolution should be like:
> 
> - 	WARN_ON_ONCE(!page_has_buffers(page));
> + 	WARN_ON_ONCE(!folio_buffers(folio));

Thanks, I have updated "git rerere"s resolution for tomorrow.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the folio tree with the ext4 tree

[Theodore Ts'o]

On Tue, Mar 15, 2022 at 12:25:12PM +0100, Jan Kara wrote:
> Thanks for the notice Stephen! The resolution should be like:
> 
> - 	WARN_ON_ONCE(!page_has_buffers(page));
> + 	WARN_ON_ONCE(!folio_buffers(folio));
> 
> How are we going to handle this conflict Ted? Just tell Linus about the
> conflict and resolution?

Agreed, I think that's the simplest approach.

					- Ted