Re: [PATCH 13/29] crypto: scatterwalk - add new functions for iterating through data

[Eric Biggers <ebiggers@kernel.org>]

On Sat, Dec 21, 2024 at 07:27:30PM +0800, Herbert Xu wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > +       if (ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE)
> > +               flush_dcache_page(scatterwalk_page(walk));
> 
> Does the if statement do anything? If so please add a comment
> about what it does because it's not obvious.
> 

Yes, since even though flush_dcache_page() is a no-op when
!ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, scatterwalk_page() still calls sg_page()
which calls BUG_ON() when CONFIG_DEBUG_SG=y.  That can't be optimized out.  Note
that despite ostensibly being a debug option, CONFIG_DEBUG_SG=y is often used in
production kernels.  Also, in patch 29 this changes into a loop that may flush
multiple pages, and which makes it harder to show that it would get optimized
out even with CONFIG_DEBUG_SG=n; e.g., the compiler would have to prove that the
loop is finite.  I will add a comment to this patch and update it in patch 29.

- Eric