Miao Xie
2010-Mar-25 12:27 UTC
[PATCH 01/18] btrfs: Remove u64 conversion for PAGE_CACHE_SIZE
From: Zhao Lei <zhaolei@cn.fujitsu.com> We don''t need to convert PAGE_CACHE_SIZE to u64 in bit operation. Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> --- fs/btrfs/compression.c | 2 +- fs/btrfs/extent_io.c | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index 28b92a7..467e631 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -349,7 +349,7 @@ int btrfs_submit_compressed_write(struct inode *inode, u64 start, struct block_device *bdev; int ret; - WARN_ON(start & ((u64)PAGE_CACHE_SIZE - 1)); + WARN_ON(start & (PAGE_CACHE_SIZE - 1)); cb = kmalloc(compressed_bio_size(root, compressed_len), GFP_NOFS); atomic_set(&cb->pending_bios, 0); cb->errors = 0; diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index c99121a..bdfbfa6 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3489,7 +3489,7 @@ void read_extent_buffer(struct extent_buffer *eb, void *dstv, struct page *page; char *kaddr; char *dst = (char *)dstv; - size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); + size_t start_offset = eb->start & (PAGE_CACHE_SIZE - 1); unsigned long i = (start_offset + start) >> PAGE_CACHE_SHIFT; WARN_ON(start > eb->len); @@ -3520,7 +3520,7 @@ int map_private_extent_buffer(struct extent_buffer *eb, unsigned long start, size_t offset = start & (PAGE_CACHE_SIZE - 1); char *kaddr; struct page *p; - size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); + size_t start_offset = eb->start & (PAGE_CACHE_SIZE - 1); unsigned long i = (start_offset + start) >> PAGE_CACHE_SHIFT; unsigned long end_i = (start_offset + start + min_len - 1) >> PAGE_CACHE_SHIFT; @@ -3589,7 +3589,7 @@ int memcmp_extent_buffer(struct extent_buffer *eb, const void *ptrv, struct page *page; char *kaddr; char *ptr = (char *)ptrv; - size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); + size_t start_offset = eb->start & (PAGE_CACHE_SIZE - 1); unsigned long i = (start_offset + start) >> PAGE_CACHE_SHIFT; int ret = 0; @@ -3625,7 +3625,7 @@ void write_extent_buffer(struct extent_buffer *eb, const void *srcv, struct page *page; char *kaddr; char *src = (char *)srcv; - size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); + size_t start_offset = eb->start & (PAGE_CACHE_SIZE - 1); unsigned long i = (start_offset + start) >> PAGE_CACHE_SHIFT; WARN_ON(start > eb->len); @@ -3656,7 +3656,7 @@ void memset_extent_buffer(struct extent_buffer *eb, char c, size_t offset; struct page *page; char *kaddr; - size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); + size_t start_offset = eb->start & (PAGE_CACHE_SIZE - 1); unsigned long i = (start_offset + start) >> PAGE_CACHE_SHIFT; WARN_ON(start > eb->len); @@ -3688,7 +3688,7 @@ void copy_extent_buffer(struct extent_buffer *dst, struct extent_buffer *src, size_t offset; struct page *page; char *kaddr; - size_t start_offset = dst->start & ((u64)PAGE_CACHE_SIZE - 1); + size_t start_offset = dst->start & (PAGE_CACHE_SIZE - 1); unsigned long i = (start_offset + dst_offset) >> PAGE_CACHE_SHIFT; WARN_ON(src->len != dst_len); @@ -3757,7 +3757,7 @@ void memcpy_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset, 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); + size_t start_offset = dst->start & (PAGE_CACHE_SIZE - 1); unsigned long dst_i; unsigned long src_i; @@ -3804,7 +3804,7 @@ void memmove_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset, size_t src_off_in_page; unsigned long dst_end = dst_offset + len - 1; unsigned long src_end = src_offset + len - 1; - size_t start_offset = dst->start & ((u64)PAGE_CACHE_SIZE - 1); + size_t start_offset = dst->start & (PAGE_CACHE_SIZE - 1); unsigned long dst_i; unsigned long src_i; -- 1.6.5.2 -- 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
2010-Mar-25 13:02 UTC
Re: [PATCH 01/18] btrfs: Remove u64 conversion for PAGE_CACHE_SIZE
On Thu, Mar 25, 2010 at 08:27:26PM +0800, Miao Xie wrote:> From: Zhao Lei <zhaolei@cn.fujitsu.com> > > We don''t need to convert PAGE_CACHE_SIZE to u64 in bit operation.For code like this: u64 size = (some number that doesn''t fit in 32 bits) if (size & (PAGE_CACHE_SIZE - 1)) { } The answer should be the same either way. But if the code gets switched: start = size & ~(PAGE_CACHE_SIZE - 1); Some arches are going to get the wrong answer here. We had a few bugs like this early on and I went through and casted everything to be consistent. While this patch is correct, I would rather leave the casts to avoid subtle problems later on as the code changes. -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
Zhaolei
2010-Mar-31 04:04 UTC
Re: Re: [PATCH 01/18] btrfs: Remove u64 conversion for PAGE_CACHE_SIZE
Chris Mason wrote:> On Thu, Mar 25, 2010 at 08:27:26PM +0800, Miao Xie wrote: >> From: Zhao Lei <zhaolei@cn.fujitsu.com> >> >> We don''t need to convert PAGE_CACHE_SIZE to u64 in bit operation. > > For code like this: > > u64 size = (some number that doesn''t fit in 32 bits) > > if (size & (PAGE_CACHE_SIZE - 1)) { > } > > The answer should be the same either way. But if the code gets > switched: > > start = size & ~(PAGE_CACHE_SIZE - 1); > > Some arches are going to get the wrong answer here. We had a few bugs > like this early on and I went through and casted everything to be > consistent. While this patch is correct, I would rather leave the casts > to avoid subtle problems later on as the code changes. > > -chrisHello, chris Thanks for your explain. I got your meaning. But at least, we should make code unify: // with u64: [root@localhost btrfs]# grep ''((u64)PAGE_CACHE_SIZE - 1)'' * compression.c: WARN_ON(start & ((u64)PAGE_CACHE_SIZE - 1)); extent_io.c: size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); extent_io.c: size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); extent_io.c: size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); extent_io.c: size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); extent_io.c: size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); extent_io.c: size_t start_offset = dst->start & ((u64)PAGE_CACHE_SIZE - 1); extent_io.c: size_t start_offset = dst->start & ((u64)PAGE_CACHE_SIZE - 1); extent_io.c: size_t start_offset = dst->start & ((u64)PAGE_CACHE_SIZE - 1); // without u64: [root@localhost btrfs]# grep ''(PAGE_CACHE_SIZE - 1)'' * compression.c: size_t zero_offset = isize & (PAGE_CACHE_SIZE - 1); extent_io.c: unsigned long offset = (*start) & (PAGE_CACHE_SIZE - 1); extent_io.c: size_t zero_offset = last_byte & (PAGE_CACHE_SIZE - 1); extent_io.c: pg_offset = i_size & (PAGE_CACHE_SIZE - 1); extent_io.c: block_off_start = block_start & (PAGE_CACHE_SIZE - 1); extent_io.c: page_offset = block_start & (PAGE_CACHE_SIZE - 1); extent_io.c: if ((i == 0 && (eb->start & (PAGE_CACHE_SIZE - 1))) || extent_io.c: ((eb->start + eb->len) & (PAGE_CACHE_SIZE - 1)))) { extent_io.c: size_t offset = start & (PAGE_CACHE_SIZE - 1); file.c: int offset = pos & (PAGE_CACHE_SIZE - 1); file.c: if ((pos & (PAGE_CACHE_SIZE - 1))) { file.c: if ((pos + count) & (PAGE_CACHE_SIZE - 1)) { file.c: size_t offset = pos & (PAGE_CACHE_SIZE - 1); file-item.c: memcpy(eb_token + ((unsigned long)item & (PAGE_CACHE_SIZE - 1)), inode.c: offset = start & (PAGE_CACHE_SIZE - 1); inode.c: (PAGE_CACHE_SIZE - 1); inode.c: ~(PAGE_CACHE_SIZE - 1); inode.c: if ((end & (PAGE_CACHE_SIZE - 1)) == 0) Thanks Zhaolei> -- > 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 >-- 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