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

Eric Biggers
2024-12-23T19:53:50Z

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