linux-kernel - Re: [PATCH v2 3/6] iomap: advance the ifs allocation if we have more than one blocks per folio

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZsAqG-Q527PYWYrz@casper.infradead.org>
Date: Sat, 17 Aug 2024 05:42:03 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: Christoph Hellwig <hch@...radead.org>, linux-xfs@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	djwong@...nel.org, brauner@...nel.org, david@...morbit.com,
	jack@...e.cz, yi.zhang@...wei.com, chengzhihao1@...wei.com,
	yukuai3@...wei.com
Subject: Re: [PATCH v2 3/6] iomap: advance the ifs allocation if we have more
 than one blocks per folio

On Sat, Aug 17, 2024 at 12:27:49PM +0800, Zhang Yi wrote:
> On 2024/8/14 13:32, Christoph Hellwig wrote:
> > On Mon, Aug 12, 2024 at 08:11:56PM +0800, Zhang Yi wrote:
> >> From: Zhang Yi <yi.zhang@...wei.com>
> >>
> >> Now we allocate ifs if i_blocks_per_folio is larger than one when
> >> writing back dirty folios in iomap_writepage_map(), so we don't attach
> >> an ifs after buffer write to an entire folio until it starts writing
> >> back, if we partial truncate that folio, iomap_invalidate_folio() can't
> >> clear counterpart block's dirty bit as expected. Fix this by advance the
> >> ifs allocation to __iomap_write_begin().
> > 
> > Wouldn't it make more sense to only allocate the ifѕ in
> > iomap_invalidate_folio when it actually is needed?
> > 
> 
> I forget to mention that truncate_inode_partial_folio() call
> folio_invalidate()->iomap_invalidate_folio() only when the folio has
> private, if the folio doesn't has ifs, the iomap_invalidate_folio()
> would nerver be called, hence allocate the ifs in
> iomap_invalidate_folio() is useless.
> 
> In my opinion, one solution is change to always call folio_invalidate()
> in truncate_inode_partial_folio(), all callbacks should handle the no
> private case. Another solution is add a magic (a fake ifs) to
> folio->private and then convert it to a real one in
> iomap_invalidate_folio(), any thoughts?

Why do we need iomap_invalidate_folio() to be called if there is no ifs?
Even today, all it does is call ifs_free() if we're freeing the entire
folio (which is done by truncate_cleanup_folio() and not by
truncate_inode_partial_folio().