On Sat, Dec 21, 2024 at 07:27:30PM +0800, Herbert Xu wrote: > Eric Biggers <ebiggers@xxxxxxxxxx> 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
