| Message ID | 9c5624f2b3a0131e89f3e692553a55d132f50a96.1741688305.git.herbert@gondor.apana.org.au (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: Use nth_page instead of doing it by hand | expand |
On Tue, Mar 11, 2025 at 06:20:29PM +0800, Herbert Xu wrote: > static inline void scatterwalk_map(struct scatter_walk *walk) > { > - struct page *base_page = sg_page(walk->sg); > + struct page *page = sg_page(walk->sg); > + unsigned int offset = walk->offset; > + void *addr; > + > + page = nth_page(page, offset >> PAGE_SHIFT); > + offset = offset_in_page(offset); > > if (IS_ENABLED(CONFIG_HIGHMEM)) { > - walk->__addr = kmap_local_page(base_page + > - (walk->offset >> PAGE_SHIFT)) + > - offset_in_page(walk->offset); > + addr = kmap_local_page(page) + offset; > } else { > /* > * When !HIGHMEM we allow the walker to return segments that > * span a page boundary; see scatterwalk_clamp(). To make it > * clear that in this case we're working in the linear buffer of > * the whole sg entry in the kernel's direct map rather than > - * within the mapped buffer of a single page, compute the > - * address as an offset from the page_address() of the first > - * page of the sg entry. Either way the result is the address > - * in the direct map, but this makes it clearer what is really > - * going on. > + * within the mapped buffer of a single page, use > + * page_address() instead of going through kmap. > */ > - walk->__addr = page_address(base_page) + walk->offset; > + addr = page_address(page) + offset; > } > + walk->__addr = addr; In the !HIGHMEM case (i.e., the common case) this is just worse, though. It expands into more instructions than before, only to get the same linear address that it did before. You also seem to be ignoring the comment that explains that we're working in the linear buffer of the whole sg entry. - Eric
On Tue, Mar 11, 2025 at 10:26:24AM -0700, Eric Biggers wrote: > > In the !HIGHMEM case (i.e., the common case) this is just worse, though. It > expands into more instructions than before, only to get the same linear address > that it did before. You also seem to be ignoring the comment that explains that > we're working in the linear buffer of the whole sg entry. Thanks for letting me know. I had assumed that this would all get optimised away. Let me look into this atrocious code generation. Cheers,
diff --git a/include/crypto/scatterwalk.h b/include/crypto/scatterwalk.h index b7e617ae4442..4fc70b8422c5 100644 --- a/include/crypto/scatterwalk.h +++ b/include/crypto/scatterwalk.h @@ -99,26 +99,27 @@ static inline void scatterwalk_get_sglist(struct scatter_walk *walk, static inline void scatterwalk_map(struct scatter_walk *walk) { - struct page *base_page = sg_page(walk->sg); + struct page *page = sg_page(walk->sg); + unsigned int offset = walk->offset; + void *addr; + + page = nth_page(page, offset >> PAGE_SHIFT); + offset = offset_in_page(offset); if (IS_ENABLED(CONFIG_HIGHMEM)) { - walk->__addr = kmap_local_page(base_page + - (walk->offset >> PAGE_SHIFT)) + - offset_in_page(walk->offset); + addr = kmap_local_page(page) + offset; } else { /* * When !HIGHMEM we allow the walker to return segments that * span a page boundary; see scatterwalk_clamp(). To make it * clear that in this case we're working in the linear buffer of * the whole sg entry in the kernel's direct map rather than - * within the mapped buffer of a single page, compute the - * address as an offset from the page_address() of the first - * page of the sg entry. Either way the result is the address - * in the direct map, but this makes it clearer what is really - * going on. + * within the mapped buffer of a single page, use + * page_address() instead of going through kmap. */ - walk->__addr = page_address(base_page) + walk->offset; + addr = page_address(page) + offset; } + walk->__addr = addr; } /**
Curiously, the Crypto API scatterwalk incremented pages by hand rather than using nth_page. Possibly because scatterwalk predates nth_page (the following commit is from the history tree): commit 3957f2b34960d85b63e814262a8be7d5ad91444d Author: James Morris <jmorris@intercode.com.au> Date: Sun Feb 2 07:35:32 2003 -0800 [CRYPTO]: in/out scatterlist support for ciphers. Fix this by using nth_page. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> --- include/crypto/scatterwalk.h | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)