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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220315112512.yl7dewzglimjurh5@quack3.lan>
Date:   Tue, 15 Mar 2022 12:25:12 +0100
From:   Jan Kara <jack@...e.cz>
To:     Stephen Rothwell <sfr@...b.auug.org.au>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Theodore Ts'o <tytso@....edu>, Jan Kara <jack@...e.cz>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Next Mailing List <linux-next@...r.kernel.org>
Subject: Re: linux-next: manual merge of the folio tree with the ext4 tree

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@...e.com>
SUSE Labs, CR