On 2/9/21 22:25, ira.weiny@xxxxxxxxx wrote: > From: Ira Weiny <ira.weiny@xxxxxxxxx> > > Working through a conversion to a call kmap_local_page() instead of > kmap() revealed many places where the pattern kmap/memcpy/kunmap > occurred. > > Eric Biggers, Matthew Wilcox, Christoph Hellwig, Dan Williams, and Al > Viro all suggested putting this code into helper functions. Al Viro > further pointed out that these functions already existed in the iov_iter > code.[1] > > Various locations for the lifted functions were considered. > > Headers like mm.h or string.h seem ok but don't really portray the > functionality well. pagemap.h made some sense but is for page cache > functionality.[2] > > Another alternative would be to create a new header for the promoted > memcpy functions, but it masks the fact that these are designed to copy > to/from pages using the kernel direct mappings and complicates matters > with a new header. > > Placing these functions in 'highmem.h' is suboptimal especially with the > changes being proposed in the functionality of kmap. From a caller > perspective including/using 'highmem.h' implies that the functions > defined in that header are only required when highmem is in use which is > increasingly not the case with modern processors. However, highmem.h is > where all the current functions like this reside (zero_user(), > clear_highpage(), clear_user_highpage(), copy_user_highpage(), and > copy_highpage()). So it makes the most sense even though it is > distasteful for some.[3] > > Lift memcpy_to_page() and memcpy_from_page() to pagemap.h. > > [1] https://lore.kernel.org/lkml/20201013200149.GI3576660@xxxxxxxxxxxxxxxxxx/ > https://lore.kernel.org/lkml/20201013112544.GA5249@xxxxxxxxxxxxx/ > > [2] https://lore.kernel.org/lkml/20201208122316.GH7338@xxxxxxxxxxxxxxxxxxxx/ > > [3] https://lore.kernel.org/lkml/20201013200149.GI3576660@xxxxxxxxxxxxxxxxxx/#t > https://lore.kernel.org/lkml/20201208163814.GN1563847@xxxxxxxxxxxxxxxxxxxxxxxxx/ > > Cc: Boris Pismenny <borisp@xxxxxxxxxxxx> > Cc: Or Gerlitz <gerlitz.or@xxxxxxxxx> > Cc: Dave Hansen <dave.hansen@xxxxxxxxx> > Suggested-by: Matthew Wilcox <willy@xxxxxxxxxxxxx> > Suggested-by: Christoph Hellwig <hch@xxxxxxxxxxxxx> > Suggested-by: Dan Williams <dan.j.williams@xxxxxxxxx> > Suggested-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx> > Suggested-by: Eric Biggers <ebiggers@xxxxxxxxxx> > Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx> Thanks for adding a new line in the new calls after variable declaration. Looks good. Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
