David Sterba
2013-Apr-29 13:38 UTC
[PATCH] btrfs: use only memmove_extent_buffer and simplify the helpers
After commit a65917156e34594 ("Btrfs: stop using highmem for
extent_buffers") we don''t need to call kmap_atomic anymore and can
reduce the move_pages helper to a simple memmove.
There''s only one caller of memcpy_extent_buffer, we can use the
memmove_ variant here.
Signed-off-by: David Sterba <dsterba@suse.cz>
---
fs/btrfs/ctree.c | 2 +-
fs/btrfs/extent_io.c | 88 +------------------------------------------------
2 files changed, 3 insertions(+), 87 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 566d99b..154573e 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -4270,7 +4270,7 @@ int btrfs_duplicate_item(struct btrfs_trans_handle *trans,
item_size, item_size +
sizeof(struct btrfs_item), 1);
leaf = path->nodes[0];
- memcpy_extent_buffer(leaf,
+ memmove_extent_buffer(leaf,
btrfs_item_ptr_offset(leaf, path->slots[0]),
btrfs_item_ptr_offset(leaf, path->slots[0] - 1),
item_size);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index dbb2bde..e1abebd 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4943,89 +4943,9 @@ static void move_pages(struct page *dst_page, struct page
*src_page,
unsigned long len)
{
char *dst_kaddr = page_address(dst_page);
- if (dst_page == src_page) {
- memmove(dst_kaddr + dst_off, dst_kaddr + src_off, len);
- } else {
- char *src_kaddr = page_address(src_page);
- char *p = dst_kaddr + dst_off + len;
- char *s = src_kaddr + src_off + len;
+ char *src_kaddr = page_address(src_page);
- while (len--)
- *--p = *--s;
- }
-}
-
-static inline bool areas_overlap(unsigned long src, unsigned long dst, unsigned
long len)
-{
- unsigned long distance = (src > dst) ? src - dst : dst - src;
- return distance < len;
-}
-
-static void copy_pages(struct page *dst_page, struct page *src_page,
- unsigned long dst_off, unsigned long src_off,
- unsigned long len)
-{
- char *dst_kaddr = page_address(dst_page);
- char *src_kaddr;
- int must_memmove = 0;
-
- if (dst_page != src_page) {
- src_kaddr = page_address(src_page);
- } else {
- src_kaddr = dst_kaddr;
- if (areas_overlap(src_off, dst_off, len))
- must_memmove = 1;
- }
-
- if (must_memmove)
- memmove(dst_kaddr + dst_off, src_kaddr + src_off, len);
- else
- memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len);
-}
-
-void memcpy_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset,
- unsigned long src_offset, unsigned long len)
-{
- size_t cur;
- size_t dst_off_in_page;
- size_t src_off_in_page;
- size_t start_offset = dst->start & ((u64)PAGE_CACHE_SIZE - 1);
- unsigned long dst_i;
- unsigned long src_i;
-
- if (src_offset + len > dst->len) {
- printk(KERN_ERR "btrfs memmove bogus src_offset %lu move "
- "len %lu dst len %lu\n", src_offset, len, dst->len);
- BUG_ON(1);
- }
- if (dst_offset + len > dst->len) {
- printk(KERN_ERR "btrfs memmove bogus dst_offset %lu move "
- "len %lu dst len %lu\n", dst_offset, len, dst->len);
- BUG_ON(1);
- }
-
- while (len > 0) {
- dst_off_in_page = (start_offset + dst_offset) &
- ((unsigned long)PAGE_CACHE_SIZE - 1);
- src_off_in_page = (start_offset + src_offset) &
- ((unsigned long)PAGE_CACHE_SIZE - 1);
-
- dst_i = (start_offset + dst_offset) >> PAGE_CACHE_SHIFT;
- src_i = (start_offset + src_offset) >> PAGE_CACHE_SHIFT;
-
- cur = min(len, (unsigned long)(PAGE_CACHE_SIZE -
- src_off_in_page));
- cur = min_t(unsigned long, cur,
- (unsigned long)(PAGE_CACHE_SIZE - dst_off_in_page));
-
- copy_pages(extent_buffer_page(dst, dst_i),
- extent_buffer_page(dst, src_i),
- dst_off_in_page, src_off_in_page, cur);
-
- src_offset += cur;
- dst_offset += cur;
- len -= cur;
- }
+ memmove(dst_kaddr + dst_off, src_kaddr + src_off, len);
}
void memmove_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset,
@@ -5050,10 +4970,6 @@ void memmove_extent_buffer(struct extent_buffer *dst,
unsigned long dst_offset,
"len %lu len %lu\n", dst_offset, len, dst->len);
BUG_ON(1);
}
- if (dst_offset < src_offset) {
- memcpy_extent_buffer(dst, dst_offset, src_offset, len);
- return;
- }
while (len > 0) {
dst_i = (start_offset + dst_end) >> PAGE_CACHE_SHIFT;
src_i = (start_offset + src_end) >> PAGE_CACHE_SHIFT;
--
1.7.9
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Josef Bacik
2013-May-03 20:33 UTC
Re: [PATCH] btrfs: use only memmove_extent_buffer and simplify the helpers
On Mon, Apr 29, 2013 at 07:38:01AM -0600, David Sterba wrote:> After commit a65917156e34594 ("Btrfs: stop using highmem for > extent_buffers") we don''t need to call kmap_atomic anymore and can > reduce the move_pages helper to a simple memmove. > > There''s only one caller of memcpy_extent_buffer, we can use the > memmove_ variant here. >This makes -l 64k blow the hell up, just try generic/001. I''m kicking this patch out. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Chris Mason
2013-May-03 20:37 UTC
Re: [PATCH] btrfs: use only memmove_extent_buffer and simplify the helpers
Quoting Josef Bacik (2013-05-03 16:33:44)> On Mon, Apr 29, 2013 at 07:38:01AM -0600, David Sterba wrote: > > After commit a65917156e34594 ("Btrfs: stop using highmem for > > extent_buffers") we don''t need to call kmap_atomic anymore and can > > reduce the move_pages helper to a simple memmove. > > > > There''s only one caller of memcpy_extent_buffer, we can use the > > memmove_ variant here. > > > > This makes -l 64k blow the hell up, just try generic/001. I''m kicking this > patch out. Thanks,Sorry Dave, I only now remember having this same problem the last time I tried to get rid of memcpy. -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Sterba
2013-May-07 12:13 UTC
Re: [PATCH] btrfs: use only memmove_extent_buffer and simplify the helpers
On Fri, May 03, 2013 at 04:37:41PM -0400, Chris Mason wrote:> Quoting Josef Bacik (2013-05-03 16:33:44) > > On Mon, Apr 29, 2013 at 07:38:01AM -0600, David Sterba wrote: > > > After commit a65917156e34594 ("Btrfs: stop using highmem for > > > extent_buffers") we don''t need to call kmap_atomic anymore and can > > > reduce the move_pages helper to a simple memmove. > > > > > > There''s only one caller of memcpy_extent_buffer, we can use the > > > memmove_ variant here. > > > > > > > This makes -l 64k blow the hell up, just try generic/001. I''m kicking this > > patch out. Thanks, > > Sorry Dave, I only now remember having this same problem the last time I > tried to get rid of memcpy.The patch evolved from a minor cleanup inside copy_page into a functional change that looked ok to me after checking that memmove behaves like memcpy if it''s safe (both generic and arch-specific implementations). Well, apparently not, but I still see some space for micro-optimizations around copy_pages/move_pages. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html