* [PATCH 01/18] btrfs: Remove u64 conversion for PAGE_CACHE_SIZE
@ 2010-03-25 12:27 Miao Xie
2010-03-25 13:02 ` Chris Mason
0 siblings, 1 reply; 3+ messages in thread
From: Miao Xie @ 2010-03-25 12:27 UTC (permalink / raw)
To: Chris Mason; +Cc: Linux Btrfs
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 01/18] btrfs: Remove u64 conversion for PAGE_CACHE_SIZE
2010-03-25 12:27 [PATCH 01/18] btrfs: Remove u64 conversion for PAGE_CACHE_SIZE Miao Xie
@ 2010-03-25 13:02 ` Chris Mason
2010-03-31 4:04 ` Zhaolei
0 siblings, 1 reply; 3+ messages in thread
From: Chris Mason @ 2010-03-25 13:02 UTC (permalink / raw)
To: Miao Xie; +Cc: Linux Btrfs
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: Re: [PATCH 01/18] btrfs: Remove u64 conversion for PAGE_CACHE_SIZE
2010-03-25 13:02 ` Chris Mason
@ 2010-03-31 4:04 ` Zhaolei
0 siblings, 0 replies; 3+ messages in thread
From: Zhaolei @ 2010-03-31 4:04 UTC (permalink / raw)
To: Chris Mason, Miao Xie, Linux Btrfs
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.
>
> -chris
Hello, 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-03-31 4:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-25 12:27 [PATCH 01/18] btrfs: Remove u64 conversion for PAGE_CACHE_SIZE Miao Xie
2010-03-25 13:02 ` Chris Mason
2010-03-31 4:04 ` Zhaolei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).