Re: [PATCH v14 05/21] fsverity: improve flushing performance of fsverity_fill_zerohash

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Aug 04, 2026 at 06:57:00PM +0000, Eric Biggers wrote:
> On Tue, Aug 04, 2026 at 06:37:52PM +0000, Eric Biggers wrote:
> > On Tue, Aug 04, 2026 at 07:42:23PM +0200, Christoph Hellwig wrote:
> > > On Mon, Aug 03, 2026 at 10:07:55PM +0200, Andrey Albershteyn wrote:
> > > > The current version calls flush_dcache_folio(), in memcpy_to_folio(), to
> > > > flush whole folio on every digest (which is 128 for 4k) on the HIGHMEM
> > > > systems. Open code folio mapping and flushing to copy all digests at
> > > > once.
> > > 
> > > This looks correct, although to me optimizing for this feels like
> > > premature optimizations not worth the ugly code unless we have numbers
> > > to justify it.
> > > 
> > > If Eric wants it:
> > > 
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > > +		if (folio_test_partial_kmap(folio) &&
> > > > +		    off > PAGE_SIZE - offset_in_page(offset))
> > > > +			off = PAGE_SIZE - offset_in_page(offset);
> > > > +		for (; to < (vaddr + off); to += vi->tree_params.digest_size)
> > > 
> > > Style nitpick: no need for braces when comparing with simple
> > > integer arithmetics like this.
> > > 
> > > > +	for (off = offset; off < (offset + len);
> > > 
> > > Same here.
> > 
> > Well I didn't ask for it per se, but I pointed it out and asked whether
> > anyone will care about the combination of XFS && FS_VERITY && HIGHMEM.
> > Based on Darrick's email it seems the answer may be no?
> > 
> > If it's kept as-is, adding a comment mentioning that it doesn't need to
> > be optimized for HIGHMEM would help preempt any questions about it.
> 
> Note that the explanation in the commit message seems to be confusing
> people as well.  It only mentions flush_dcache_folio(), when the actual
> performance problem on HIGHMEM would be the mapping and unmapping.

Ah.  In that case I definitely don't care to optimize it unless we get a
complaint from a real user.  XFS doesn't really support 32-bit anymore
because xfs_repair on large filesystems is known to run out of address
space for all of its temporary indexes and crash.

(I'd be fine with dropping this entirely.)

((Yes, we could increase the amount of address space by cheating with
memfds, but yuck.))

--D

> - Eric
>