| Message ID | 20220606141533.555-1-fmdefrancesco@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/highmem: Delete memmove_page() | expand |
On 06/06/22 at 04:15pm, Fabio M. De Francesco wrote: > Matthew Wilcox reported that, while he was looking at memmove_page(), he > realized that it can't actually work. > > The reasons are hidden in its implementation, which makes use of > memmove() on logical addresses provided by kmap_local_page(). memmove() > does the wrong thing when it tests "if (dest <= src)". > > Therefore, delete memmove_page(). > > No need to change any other code because we have no call sites of > memmove_page() across the whole kernel. Nice clean up. Reviewed-by: Baoquan He <bhe@redhat.com> > > Reported-by: Matthew Wilcox <willy@infradead.org> > Cc: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > --- > include/linux/highmem.h | 13 ------------- > 1 file changed, 13 deletions(-) > > diff --git a/include/linux/highmem.h b/include/linux/highmem.h > index 3af34de54330..fee9835e3793 100644 > --- a/include/linux/highmem.h > +++ b/include/linux/highmem.h > @@ -336,19 +336,6 @@ static inline void memcpy_page(struct page *dst_page, size_t dst_off, > kunmap_local(dst); > } > > -static inline void memmove_page(struct page *dst_page, size_t dst_off, > - struct page *src_page, size_t src_off, > - size_t len) > -{ > - char *dst = kmap_local_page(dst_page); > - char *src = kmap_local_page(src_page); > - > - VM_BUG_ON(dst_off + len > PAGE_SIZE || src_off + len > PAGE_SIZE); > - memmove(dst + dst_off, src + src_off, len); > - kunmap_local(src); > - kunmap_local(dst); > -} > - > static inline void memset_page(struct page *page, size_t offset, int val, > size_t len) > { > -- > 2.36.1 > >
On Mon, Jun 06, 2022 at 04:15:33PM +0200, Fabio M. De Francesco wrote: > Matthew Wilcox reported that, while he was looking at memmove_page(), he > realized that it can't actually work. > > The reasons are hidden in its implementation, which makes use of > memmove() on logical addresses provided by kmap_local_page(). memmove() > does the wrong thing when it tests "if (dest <= src)". > > Therefore, delete memmove_page(). > > No need to change any other code because we have no call sites of > memmove_page() across the whole kernel. > > Reported-by: Matthew Wilcox <willy@infradead.org> > Cc: Ira Weiny <ira.weiny@intel.com> Reviewed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > --- > include/linux/highmem.h | 13 ------------- > 1 file changed, 13 deletions(-) > > diff --git a/include/linux/highmem.h b/include/linux/highmem.h > index 3af34de54330..fee9835e3793 100644 > --- a/include/linux/highmem.h > +++ b/include/linux/highmem.h > @@ -336,19 +336,6 @@ static inline void memcpy_page(struct page *dst_page, size_t dst_off, > kunmap_local(dst); > } > > -static inline void memmove_page(struct page *dst_page, size_t dst_off, > - struct page *src_page, size_t src_off, > - size_t len) > -{ > - char *dst = kmap_local_page(dst_page); > - char *src = kmap_local_page(src_page); > - > - VM_BUG_ON(dst_off + len > PAGE_SIZE || src_off + len > PAGE_SIZE); > - memmove(dst + dst_off, src + src_off, len); > - kunmap_local(src); > - kunmap_local(dst); > -} > - > static inline void memset_page(struct page *page, size_t offset, int val, > size_t len) > { > -- > 2.36.1 >
diff --git a/include/linux/highmem.h b/include/linux/highmem.h index 3af34de54330..fee9835e3793 100644 --- a/include/linux/highmem.h +++ b/include/linux/highmem.h @@ -336,19 +336,6 @@ static inline void memcpy_page(struct page *dst_page, size_t dst_off, kunmap_local(dst); } -static inline void memmove_page(struct page *dst_page, size_t dst_off, - struct page *src_page, size_t src_off, - size_t len) -{ - char *dst = kmap_local_page(dst_page); - char *src = kmap_local_page(src_page); - - VM_BUG_ON(dst_off + len > PAGE_SIZE || src_off + len > PAGE_SIZE); - memmove(dst + dst_off, src + src_off, len); - kunmap_local(src); - kunmap_local(dst); -} - static inline void memset_page(struct page *page, size_t offset, int val, size_t len) {
Matthew Wilcox reported that, while he was looking at memmove_page(), he realized that it can't actually work. The reasons are hidden in its implementation, which makes use of memmove() on logical addresses provided by kmap_local_page(). memmove() does the wrong thing when it tests "if (dest <= src)". Therefore, delete memmove_page(). No need to change any other code because we have no call sites of memmove_page() across the whole kernel. Reported-by: Matthew Wilcox <willy@infradead.org> Cc: Ira Weiny <ira.weiny@intel.com> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- include/linux/highmem.h | 13 ------------- 1 file changed, 13 deletions(-)