* [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED
@ 2018-12-05 14:23 Johannes Thumshirn
2018-12-05 14:23 ` [PATCH 1/3] btrfs: use offset_in_page instead of open-coding it Johannes Thumshirn
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Johannes Thumshirn @ 2018-12-05 14:23 UTC (permalink / raw)
To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn
Use the offset_in_page() and PAGE_ALIGNED() macros instead of open-coding them
throughout btrfs.
This series also includes a patch for 'make coccicheck' which is marked as an
RFC and I've CCed Julia in the hoping to get input from her.
Johannes Thumshirn (3):
btrfs: use offset_in_page instead of open-coding it
btrfs: use PAGE_ALIGNED instead of open-coding it
coccinelle: api: add offset_in_page.cocci
fs/btrfs/check-integrity.c | 20 +++----
fs/btrfs/compression.c | 4 +-
fs/btrfs/extent_io.c | 53 +++++++++----------
fs/btrfs/file.c | 4 +-
fs/btrfs/inode.c | 9 ++--
fs/btrfs/send.c | 2 +-
fs/btrfs/volumes.c | 2 +-
scripts/coccinelle/api/offset_in_page.cocci | 81 +++++++++++++++++++++++++++++
8 files changed, 125 insertions(+), 50 deletions(-)
create mode 100644 scripts/coccinelle/api/offset_in_page.cocci
--
2.16.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] btrfs: use offset_in_page instead of open-coding it 2018-12-05 14:23 [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED Johannes Thumshirn @ 2018-12-05 14:23 ` Johannes Thumshirn 2018-12-05 14:32 ` Nikolay Borisov 2018-12-05 14:23 ` [PATCH 2/3] btrfs: use PAGE_ALIGNED " Johannes Thumshirn ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Johannes Thumshirn @ 2018-12-05 14:23 UTC (permalink / raw) To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn Constructs like 'var & (PAGE_SIZE - 1)' or 'var & ~PAGE_MASK' can denote an offset into a page. So replace them by the offset_in_page() macro instead of open-coding it if they're not used as an alignment check. Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> --- fs/btrfs/check-integrity.c | 12 +++++------ fs/btrfs/compression.c | 2 +- fs/btrfs/extent_io.c | 53 +++++++++++++++++++++------------------------- fs/btrfs/file.c | 4 ++-- fs/btrfs/inode.c | 7 +++--- fs/btrfs/send.c | 2 +- fs/btrfs/volumes.c | 2 +- 7 files changed, 38 insertions(+), 44 deletions(-) diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c index 781cae168d2a..d319c3020c09 100644 --- a/fs/btrfs/check-integrity.c +++ b/fs/btrfs/check-integrity.c @@ -1202,24 +1202,24 @@ static void btrfsic_read_from_block_data( void *dstv, u32 offset, size_t len) { size_t cur; - size_t offset_in_page; + size_t pgoff; char *kaddr; char *dst = (char *)dstv; - size_t start_offset = block_ctx->start & ((u64)PAGE_SIZE - 1); + size_t start_offset = offset_in_page(block_ctx->start); unsigned long i = (start_offset + offset) >> PAGE_SHIFT; WARN_ON(offset + len > block_ctx->len); - offset_in_page = (start_offset + offset) & (PAGE_SIZE - 1); + pgoff = offset_in_page(start_offset + offset); while (len > 0) { - cur = min(len, ((size_t)PAGE_SIZE - offset_in_page)); + cur = min(len, ((size_t)PAGE_SIZE - pgoff)); BUG_ON(i >= DIV_ROUND_UP(block_ctx->len, PAGE_SIZE)); kaddr = block_ctx->datav[i]; - memcpy(dst, kaddr + offset_in_page, cur); + memcpy(dst, kaddr + pgoff, cur); dst += cur; len -= cur; - offset_in_page = 0; + pgoff = 0; i++; } } diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index dba59ae914b8..2ab5591449f2 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -477,7 +477,7 @@ static noinline int add_ra_bio_pages(struct inode *inode, if (page->index == end_index) { char *userpage; - size_t zero_offset = isize & (PAGE_SIZE - 1); + size_t zero_offset = offset_in_page(isize); if (zero_offset) { int zeros; diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index b2769e92b556..e365c5272e6b 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -2585,7 +2585,7 @@ static void end_bio_extent_readpage(struct bio *bio) unsigned off; /* Zero out the end if this page straddles i_size */ - off = i_size & (PAGE_SIZE-1); + off = offset_in_page(i_size); if (page->index == end_index && off) zero_user_segment(page, off, PAGE_SIZE); SetPageUptodate(page); @@ -2888,7 +2888,7 @@ static int __do_readpage(struct extent_io_tree *tree, if (page->index == last_byte >> PAGE_SHIFT) { char *userpage; - size_t zero_offset = last_byte & (PAGE_SIZE - 1); + size_t zero_offset = offset_in_page(last_byte); if (zero_offset) { iosize = PAGE_SIZE - zero_offset; @@ -3432,7 +3432,7 @@ static int __extent_writepage(struct page *page, struct writeback_control *wbc, ClearPageError(page); - pg_offset = i_size & (PAGE_SIZE - 1); + pg_offset = offset_in_page(i_size); if (page->index > end_index || (page->index == end_index && !pg_offset)) { page->mapping->a_ops->invalidatepage(page, 0, PAGE_SIZE); @@ -5307,7 +5307,7 @@ void read_extent_buffer(const struct extent_buffer *eb, void *dstv, struct page *page; char *kaddr; char *dst = (char *)dstv; - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); + size_t start_offset = offset_in_page(eb->start); unsigned long i = (start_offset + start) >> PAGE_SHIFT; if (start + len > eb->len) { @@ -5317,7 +5317,7 @@ void read_extent_buffer(const struct extent_buffer *eb, void *dstv, return; } - offset = (start_offset + start) & (PAGE_SIZE - 1); + offset = offset_in_page(start_offset + start); while (len > 0) { page = eb->pages[i]; @@ -5342,14 +5342,14 @@ int read_extent_buffer_to_user(const struct extent_buffer *eb, struct page *page; char *kaddr; char __user *dst = (char __user *)dstv; - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); + size_t start_offset = offset_in_page(eb->start); unsigned long i = (start_offset + start) >> PAGE_SHIFT; int ret = 0; WARN_ON(start > eb->len); WARN_ON(start + len > eb->start + eb->len); - offset = (start_offset + start) & (PAGE_SIZE - 1); + offset = offset_in_page(start_offset + start); while (len > 0) { page = eb->pages[i]; @@ -5383,7 +5383,7 @@ int map_private_extent_buffer(const struct extent_buffer *eb, size_t offset; char *kaddr; struct page *p; - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); + size_t start_offset = offset_in_page(eb->start); unsigned long i = (start_offset + start) >> PAGE_SHIFT; unsigned long end_i = (start_offset + start + min_len - 1) >> PAGE_SHIFT; @@ -5420,14 +5420,14 @@ int memcmp_extent_buffer(const struct extent_buffer *eb, const void *ptrv, struct page *page; char *kaddr; char *ptr = (char *)ptrv; - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); + size_t start_offset = offset_in_page(eb->start); unsigned long i = (start_offset + start) >> PAGE_SHIFT; int ret = 0; WARN_ON(start > eb->len); WARN_ON(start + len > eb->start + eb->len); - offset = (start_offset + start) & (PAGE_SIZE - 1); + offset = offset_in_page(start_offset + start); while (len > 0) { page = eb->pages[i]; @@ -5476,13 +5476,13 @@ 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_SIZE - 1); + size_t start_offset = offset_in_page(eb->start); unsigned long i = (start_offset + start) >> PAGE_SHIFT; WARN_ON(start > eb->len); WARN_ON(start + len > eb->start + eb->len); - offset = (start_offset + start) & (PAGE_SIZE - 1); + offset = offset_in_page(start_offset + start); while (len > 0) { page = eb->pages[i]; @@ -5506,13 +5506,13 @@ void memzero_extent_buffer(struct extent_buffer *eb, unsigned long start, size_t offset; struct page *page; char *kaddr; - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); + size_t start_offset = offset_in_page(eb->start); unsigned long i = (start_offset + start) >> PAGE_SHIFT; WARN_ON(start > eb->len); WARN_ON(start + len > eb->start + eb->len); - offset = (start_offset + start) & (PAGE_SIZE - 1); + offset = offset_in_page(start_offset + start); while (len > 0) { page = eb->pages[i]; @@ -5551,13 +5551,12 @@ 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_SIZE - 1); + size_t start_offset = offset_in_page(dst->start); unsigned long i = (start_offset + dst_offset) >> PAGE_SHIFT; WARN_ON(src->len != dst_len); - offset = (start_offset + dst_offset) & - (PAGE_SIZE - 1); + offset = offset_in_page(start_offset + dst_offset); while (len > 0) { page = dst->pages[i]; @@ -5593,7 +5592,7 @@ static inline void eb_bitmap_offset(struct extent_buffer *eb, unsigned long *page_index, size_t *page_offset) { - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); + size_t start_offset = offset_in_page(eb->start); size_t byte_offset = BIT_BYTE(nr); size_t offset; @@ -5605,7 +5604,7 @@ static inline void eb_bitmap_offset(struct extent_buffer *eb, offset = start_offset + start + byte_offset; *page_index = offset >> PAGE_SHIFT; - *page_offset = offset & (PAGE_SIZE - 1); + *page_offset = offset_in_page(offset); } /** @@ -5747,7 +5746,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_SIZE - 1); + size_t start_offset = offset_in_page(dst->start); unsigned long dst_i; unsigned long src_i; @@ -5765,10 +5764,8 @@ void memcpy_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset, } while (len > 0) { - dst_off_in_page = (start_offset + dst_offset) & - (PAGE_SIZE - 1); - src_off_in_page = (start_offset + src_offset) & - (PAGE_SIZE - 1); + dst_off_in_page = offset_in_page(start_offset + dst_offset); + src_off_in_page = offset_in_page(start_offset + src_offset); dst_i = (start_offset + dst_offset) >> PAGE_SHIFT; src_i = (start_offset + src_offset) >> PAGE_SHIFT; @@ -5796,7 +5793,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_SIZE - 1); + size_t start_offset = offset_in_page(dst->start); unsigned long dst_i; unsigned long src_i; @@ -5820,10 +5817,8 @@ void memmove_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset, dst_i = (start_offset + dst_end) >> PAGE_SHIFT; src_i = (start_offset + src_end) >> PAGE_SHIFT; - dst_off_in_page = (start_offset + dst_end) & - (PAGE_SIZE - 1); - src_off_in_page = (start_offset + src_end) & - (PAGE_SIZE - 1); + dst_off_in_page = offset_in_page(start_offset + dst_end); + src_off_in_page = offset_in_page(start_offset + src_end); cur = min_t(unsigned long, len, src_off_in_page + 1); cur = min(cur, dst_off_in_page + 1); diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 3835bb8c146d..81aae230d1a5 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -399,7 +399,7 @@ static noinline int btrfs_copy_from_user(loff_t pos, size_t write_bytes, size_t copied = 0; size_t total_copied = 0; int pg = 0; - int offset = pos & (PAGE_SIZE - 1); + int offset = offset_in_page(pos); while (write_bytes > 0) { size_t count = min_t(size_t, @@ -1611,7 +1611,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb, return -ENOMEM; while (iov_iter_count(i) > 0) { - size_t offset = pos & (PAGE_SIZE - 1); + size_t offset = offset_in_page(pos); size_t sector_offset; size_t write_bytes = min(iov_iter_count(i), nrptrs * (size_t)PAGE_SIZE - diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index d54bdef16d8d..bc0564c384de 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -230,7 +230,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans, start >> PAGE_SHIFT); btrfs_set_file_extent_compression(leaf, ei, 0); kaddr = kmap_atomic(page); - offset = start & (PAGE_SIZE - 1); + offset = offset_in_page(start); write_extent_buffer(leaf, kaddr + offset, ptr, size); kunmap_atomic(kaddr); put_page(page); @@ -539,8 +539,7 @@ static noinline void compress_file_range(struct inode *inode, &total_compressed); if (!ret) { - unsigned long offset = total_compressed & - (PAGE_SIZE - 1); + unsigned long offset = offset_in_page(total_compressed); struct page *page = pages[nr_pages - 1]; char *kaddr; @@ -8878,7 +8877,7 @@ vm_fault_t btrfs_page_mkwrite(struct vm_fault *vmf) /* page is wholly or partially inside EOF */ if (page_start + PAGE_SIZE > size) - zero_start = size & ~PAGE_MASK; + zero_start = offset_in_page(size); else zero_start = PAGE_SIZE; diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 5be83b5a1b43..9df4c0b0e789 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -4775,7 +4775,7 @@ static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len) struct btrfs_key key; pgoff_t index = offset >> PAGE_SHIFT; pgoff_t last_index; - unsigned pg_offset = offset & ~PAGE_MASK; + unsigned pg_offset = offset_in_page(offset); ssize_t ret = 0; key.objectid = sctx->cur_ino; diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index d49baad64fe6..bb0430b2a8ba 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1436,7 +1436,7 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr, p = kmap(*page); /* align our pointer to the offset of the super block */ - *disk_super = p + (bytenr & ~PAGE_MASK); + *disk_super = p + offset_in_page(bytenr); if (btrfs_super_bytenr(*disk_super) != bytenr || btrfs_super_magic(*disk_super) != BTRFS_MAGIC) { -- 2.16.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] btrfs: use offset_in_page instead of open-coding it 2018-12-05 14:23 ` [PATCH 1/3] btrfs: use offset_in_page instead of open-coding it Johannes Thumshirn @ 2018-12-05 14:32 ` Nikolay Borisov 0 siblings, 0 replies; 10+ messages in thread From: Nikolay Borisov @ 2018-12-05 14:32 UTC (permalink / raw) To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist On 5.12.18 г. 16:23 ч., Johannes Thumshirn wrote: > Constructs like 'var & (PAGE_SIZE - 1)' or 'var & ~PAGE_MASK' can denote an > offset into a page. > > So replace them by the offset_in_page() macro instead of open-coding it if > they're not used as an alignment check. > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/check-integrity.c | 12 +++++------ > fs/btrfs/compression.c | 2 +- > fs/btrfs/extent_io.c | 53 +++++++++++++++++++++------------------------- > fs/btrfs/file.c | 4 ++-- > fs/btrfs/inode.c | 7 +++--- > fs/btrfs/send.c | 2 +- > fs/btrfs/volumes.c | 2 +- > 7 files changed, 38 insertions(+), 44 deletions(-) > > diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c > index 781cae168d2a..d319c3020c09 100644 > --- a/fs/btrfs/check-integrity.c > +++ b/fs/btrfs/check-integrity.c > @@ -1202,24 +1202,24 @@ static void btrfsic_read_from_block_data( > void *dstv, u32 offset, size_t len) > { > size_t cur; > - size_t offset_in_page; > + size_t pgoff; > char *kaddr; > char *dst = (char *)dstv; > - size_t start_offset = block_ctx->start & ((u64)PAGE_SIZE - 1); > + size_t start_offset = offset_in_page(block_ctx->start); > unsigned long i = (start_offset + offset) >> PAGE_SHIFT; > > WARN_ON(offset + len > block_ctx->len); > - offset_in_page = (start_offset + offset) & (PAGE_SIZE - 1); > + pgoff = offset_in_page(start_offset + offset); > > while (len > 0) { > - cur = min(len, ((size_t)PAGE_SIZE - offset_in_page)); > + cur = min(len, ((size_t)PAGE_SIZE - pgoff)); > BUG_ON(i >= DIV_ROUND_UP(block_ctx->len, PAGE_SIZE)); > kaddr = block_ctx->datav[i]; > - memcpy(dst, kaddr + offset_in_page, cur); > + memcpy(dst, kaddr + pgoff, cur); > > dst += cur; > len -= cur; > - offset_in_page = 0; > + pgoff = 0; > i++; > } > } > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index dba59ae914b8..2ab5591449f2 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -477,7 +477,7 @@ static noinline int add_ra_bio_pages(struct inode *inode, > > if (page->index == end_index) { > char *userpage; > - size_t zero_offset = isize & (PAGE_SIZE - 1); > + size_t zero_offset = offset_in_page(isize); > > if (zero_offset) { > int zeros; > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index b2769e92b556..e365c5272e6b 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2585,7 +2585,7 @@ static void end_bio_extent_readpage(struct bio *bio) > unsigned off; > > /* Zero out the end if this page straddles i_size */ > - off = i_size & (PAGE_SIZE-1); > + off = offset_in_page(i_size); > if (page->index == end_index && off) > zero_user_segment(page, off, PAGE_SIZE); > SetPageUptodate(page); > @@ -2888,7 +2888,7 @@ static int __do_readpage(struct extent_io_tree *tree, > > if (page->index == last_byte >> PAGE_SHIFT) { > char *userpage; > - size_t zero_offset = last_byte & (PAGE_SIZE - 1); > + size_t zero_offset = offset_in_page(last_byte); > > if (zero_offset) { > iosize = PAGE_SIZE - zero_offset; > @@ -3432,7 +3432,7 @@ static int __extent_writepage(struct page *page, struct writeback_control *wbc, > > ClearPageError(page); > > - pg_offset = i_size & (PAGE_SIZE - 1); > + pg_offset = offset_in_page(i_size); > if (page->index > end_index || > (page->index == end_index && !pg_offset)) { > page->mapping->a_ops->invalidatepage(page, 0, PAGE_SIZE); > @@ -5307,7 +5307,7 @@ void read_extent_buffer(const struct extent_buffer *eb, void *dstv, > struct page *page; > char *kaddr; > char *dst = (char *)dstv; > - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); > + size_t start_offset = offset_in_page(eb->start); > unsigned long i = (start_offset + start) >> PAGE_SHIFT; > > if (start + len > eb->len) { > @@ -5317,7 +5317,7 @@ void read_extent_buffer(const struct extent_buffer *eb, void *dstv, > return; > } > > - offset = (start_offset + start) & (PAGE_SIZE - 1); > + offset = offset_in_page(start_offset + start); > > while (len > 0) { > page = eb->pages[i]; > @@ -5342,14 +5342,14 @@ int read_extent_buffer_to_user(const struct extent_buffer *eb, > struct page *page; > char *kaddr; > char __user *dst = (char __user *)dstv; > - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); > + size_t start_offset = offset_in_page(eb->start); > unsigned long i = (start_offset + start) >> PAGE_SHIFT; > int ret = 0; > > WARN_ON(start > eb->len); > WARN_ON(start + len > eb->start + eb->len); > > - offset = (start_offset + start) & (PAGE_SIZE - 1); > + offset = offset_in_page(start_offset + start); > > while (len > 0) { > page = eb->pages[i]; > @@ -5383,7 +5383,7 @@ int map_private_extent_buffer(const struct extent_buffer *eb, > size_t offset; > char *kaddr; > struct page *p; > - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); > + size_t start_offset = offset_in_page(eb->start); > unsigned long i = (start_offset + start) >> PAGE_SHIFT; > unsigned long end_i = (start_offset + start + min_len - 1) >> > PAGE_SHIFT; > @@ -5420,14 +5420,14 @@ int memcmp_extent_buffer(const struct extent_buffer *eb, const void *ptrv, > struct page *page; > char *kaddr; > char *ptr = (char *)ptrv; > - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); > + size_t start_offset = offset_in_page(eb->start); > unsigned long i = (start_offset + start) >> PAGE_SHIFT; > int ret = 0; > > WARN_ON(start > eb->len); > WARN_ON(start + len > eb->start + eb->len); > > - offset = (start_offset + start) & (PAGE_SIZE - 1); > + offset = offset_in_page(start_offset + start); > > while (len > 0) { > page = eb->pages[i]; > @@ -5476,13 +5476,13 @@ 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_SIZE - 1); > + size_t start_offset = offset_in_page(eb->start); > unsigned long i = (start_offset + start) >> PAGE_SHIFT; > > WARN_ON(start > eb->len); > WARN_ON(start + len > eb->start + eb->len); > > - offset = (start_offset + start) & (PAGE_SIZE - 1); > + offset = offset_in_page(start_offset + start); > > while (len > 0) { > page = eb->pages[i]; > @@ -5506,13 +5506,13 @@ void memzero_extent_buffer(struct extent_buffer *eb, unsigned long start, > size_t offset; > struct page *page; > char *kaddr; > - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); > + size_t start_offset = offset_in_page(eb->start); > unsigned long i = (start_offset + start) >> PAGE_SHIFT; > > WARN_ON(start > eb->len); > WARN_ON(start + len > eb->start + eb->len); > > - offset = (start_offset + start) & (PAGE_SIZE - 1); > + offset = offset_in_page(start_offset + start); > > while (len > 0) { > page = eb->pages[i]; > @@ -5551,13 +5551,12 @@ 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_SIZE - 1); > + size_t start_offset = offset_in_page(dst->start); > unsigned long i = (start_offset + dst_offset) >> PAGE_SHIFT; > > WARN_ON(src->len != dst_len); > > - offset = (start_offset + dst_offset) & > - (PAGE_SIZE - 1); > + offset = offset_in_page(start_offset + dst_offset); > > while (len > 0) { > page = dst->pages[i]; > @@ -5593,7 +5592,7 @@ static inline void eb_bitmap_offset(struct extent_buffer *eb, > unsigned long *page_index, > size_t *page_offset) > { > - size_t start_offset = eb->start & ((u64)PAGE_SIZE - 1); > + size_t start_offset = offset_in_page(eb->start); > size_t byte_offset = BIT_BYTE(nr); > size_t offset; > > @@ -5605,7 +5604,7 @@ static inline void eb_bitmap_offset(struct extent_buffer *eb, > offset = start_offset + start + byte_offset; > > *page_index = offset >> PAGE_SHIFT; > - *page_offset = offset & (PAGE_SIZE - 1); > + *page_offset = offset_in_page(offset); > } > > /** > @@ -5747,7 +5746,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_SIZE - 1); > + size_t start_offset = offset_in_page(dst->start); > unsigned long dst_i; > unsigned long src_i; > > @@ -5765,10 +5764,8 @@ void memcpy_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset, > } > > while (len > 0) { > - dst_off_in_page = (start_offset + dst_offset) & > - (PAGE_SIZE - 1); > - src_off_in_page = (start_offset + src_offset) & > - (PAGE_SIZE - 1); > + dst_off_in_page = offset_in_page(start_offset + dst_offset); > + src_off_in_page = offset_in_page(start_offset + src_offset); > > dst_i = (start_offset + dst_offset) >> PAGE_SHIFT; > src_i = (start_offset + src_offset) >> PAGE_SHIFT; > @@ -5796,7 +5793,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_SIZE - 1); > + size_t start_offset = offset_in_page(dst->start); > unsigned long dst_i; > unsigned long src_i; > > @@ -5820,10 +5817,8 @@ void memmove_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset, > dst_i = (start_offset + dst_end) >> PAGE_SHIFT; > src_i = (start_offset + src_end) >> PAGE_SHIFT; > > - dst_off_in_page = (start_offset + dst_end) & > - (PAGE_SIZE - 1); > - src_off_in_page = (start_offset + src_end) & > - (PAGE_SIZE - 1); > + dst_off_in_page = offset_in_page(start_offset + dst_end); > + src_off_in_page = offset_in_page(start_offset + src_end); > > cur = min_t(unsigned long, len, src_off_in_page + 1); > cur = min(cur, dst_off_in_page + 1); > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index 3835bb8c146d..81aae230d1a5 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -399,7 +399,7 @@ static noinline int btrfs_copy_from_user(loff_t pos, size_t write_bytes, > size_t copied = 0; > size_t total_copied = 0; > int pg = 0; > - int offset = pos & (PAGE_SIZE - 1); > + int offset = offset_in_page(pos); > > while (write_bytes > 0) { > size_t count = min_t(size_t, > @@ -1611,7 +1611,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb, > return -ENOMEM; > > while (iov_iter_count(i) > 0) { > - size_t offset = pos & (PAGE_SIZE - 1); > + size_t offset = offset_in_page(pos); > size_t sector_offset; > size_t write_bytes = min(iov_iter_count(i), > nrptrs * (size_t)PAGE_SIZE - > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index d54bdef16d8d..bc0564c384de 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -230,7 +230,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans, > start >> PAGE_SHIFT); > btrfs_set_file_extent_compression(leaf, ei, 0); > kaddr = kmap_atomic(page); > - offset = start & (PAGE_SIZE - 1); > + offset = offset_in_page(start); > write_extent_buffer(leaf, kaddr + offset, ptr, size); > kunmap_atomic(kaddr); > put_page(page); > @@ -539,8 +539,7 @@ static noinline void compress_file_range(struct inode *inode, > &total_compressed); > > if (!ret) { > - unsigned long offset = total_compressed & > - (PAGE_SIZE - 1); > + unsigned long offset = offset_in_page(total_compressed); > struct page *page = pages[nr_pages - 1]; > char *kaddr; > > @@ -8878,7 +8877,7 @@ vm_fault_t btrfs_page_mkwrite(struct vm_fault *vmf) > > /* page is wholly or partially inside EOF */ > if (page_start + PAGE_SIZE > size) > - zero_start = size & ~PAGE_MASK; > + zero_start = offset_in_page(size); > else > zero_start = PAGE_SIZE; > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index 5be83b5a1b43..9df4c0b0e789 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -4775,7 +4775,7 @@ static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len) > struct btrfs_key key; > pgoff_t index = offset >> PAGE_SHIFT; > pgoff_t last_index; > - unsigned pg_offset = offset & ~PAGE_MASK; > + unsigned pg_offset = offset_in_page(offset); > ssize_t ret = 0; > > key.objectid = sctx->cur_ino; > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index d49baad64fe6..bb0430b2a8ba 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1436,7 +1436,7 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr, > p = kmap(*page); > > /* align our pointer to the offset of the super block */ > - *disk_super = p + (bytenr & ~PAGE_MASK); > + *disk_super = p + offset_in_page(bytenr); > > if (btrfs_super_bytenr(*disk_super) != bytenr || > btrfs_super_magic(*disk_super) != BTRFS_MAGIC) { > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] btrfs: use PAGE_ALIGNED instead of open-coding it 2018-12-05 14:23 [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED Johannes Thumshirn 2018-12-05 14:23 ` [PATCH 1/3] btrfs: use offset_in_page instead of open-coding it Johannes Thumshirn @ 2018-12-05 14:23 ` Johannes Thumshirn 2018-12-05 14:31 ` Nikolay Borisov 2018-12-05 14:23 ` [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci Johannes Thumshirn 2018-12-06 14:54 ` [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED David Sterba 3 siblings, 1 reply; 10+ messages in thread From: Johannes Thumshirn @ 2018-12-05 14:23 UTC (permalink / raw) To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn When using a 'var & (PAGE_SIZE - 1)' construct one is checking for a page alignment and thus should use the PAGE_ALIGNED() macro instead of open-coding it. Convert all open-coded occurrences of PAGE_ALIGNED(). Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> --- fs/btrfs/check-integrity.c | 8 ++++---- fs/btrfs/compression.c | 2 +- fs/btrfs/inode.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c index d319c3020c09..84e9729badaa 100644 --- a/fs/btrfs/check-integrity.c +++ b/fs/btrfs/check-integrity.c @@ -1601,7 +1601,7 @@ static int btrfsic_read_block(struct btrfsic_state *state, BUG_ON(block_ctx->datav); BUG_ON(block_ctx->pagev); BUG_ON(block_ctx->mem_to_free); - if (block_ctx->dev_bytenr & ((u64)PAGE_SIZE - 1)) { + if (!PAGE_ALIGNED(block_ctx->dev_bytenr)) { pr_info("btrfsic: read_block() with unaligned bytenr %llu\n", block_ctx->dev_bytenr); return -1; @@ -1778,7 +1778,7 @@ static void btrfsic_process_written_block(struct btrfsic_dev_state *dev_state, return; } is_metadata = 1; - BUG_ON(BTRFS_SUPER_INFO_SIZE & (PAGE_SIZE - 1)); + BUG_ON(!PAGE_ALIGNED(BTRFS_SUPER_INFO_SIZE)); processed_len = BTRFS_SUPER_INFO_SIZE; if (state->print_mask & BTRFSIC_PRINT_MASK_TREE_BEFORE_SB_WRITE) { @@ -2892,12 +2892,12 @@ int btrfsic_mount(struct btrfs_fs_info *fs_info, struct list_head *dev_head = &fs_devices->devices; struct btrfs_device *device; - if (fs_info->nodesize & ((u64)PAGE_SIZE - 1)) { + if (!PAGE_ALIGNED(fs_info->nodesize)) { pr_info("btrfsic: cannot handle nodesize %d not being a multiple of PAGE_SIZE %ld!\n", fs_info->nodesize, PAGE_SIZE); return -1; } - if (fs_info->sectorsize & ((u64)PAGE_SIZE - 1)) { + if (!PAGE_ALIGNED(fs_info->sectorsize)) { pr_info("btrfsic: cannot handle sectorsize %d not being a multiple of PAGE_SIZE %ld!\n", fs_info->sectorsize, PAGE_SIZE); return -1; diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index 2ab5591449f2..d5381f39a9e8 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -301,7 +301,7 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start, blk_status_t ret; int skip_sum = BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM; - WARN_ON(start & ((u64)PAGE_SIZE - 1)); + WARN_ON(!PAGE_ALIGNED(start)); cb = kmalloc(compressed_bio_size(fs_info, compressed_len), GFP_NOFS); if (!cb) return BLK_STS_RESOURCE; diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index bc0564c384de..5c52e91b01e8 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -2027,7 +2027,7 @@ int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end, unsigned int extra_bits, struct extent_state **cached_state, int dedupe) { - WARN_ON((end & (PAGE_SIZE - 1)) == 0); + WARN_ON(PAGE_ALIGNED(end)); return set_extent_delalloc(&BTRFS_I(inode)->io_tree, start, end, extra_bits, cached_state); } -- 2.16.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] btrfs: use PAGE_ALIGNED instead of open-coding it 2018-12-05 14:23 ` [PATCH 2/3] btrfs: use PAGE_ALIGNED " Johannes Thumshirn @ 2018-12-05 14:31 ` Nikolay Borisov 0 siblings, 0 replies; 10+ messages in thread From: Nikolay Borisov @ 2018-12-05 14:31 UTC (permalink / raw) To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist On 5.12.18 г. 16:23 ч., Johannes Thumshirn wrote: > When using a 'var & (PAGE_SIZE - 1)' construct one is checking for a page > alignment and thus should use the PAGE_ALIGNED() macro instead of > open-coding it. > > Convert all open-coded occurrences of PAGE_ALIGNED(). > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/check-integrity.c | 8 ++++---- > fs/btrfs/compression.c | 2 +- > fs/btrfs/inode.c | 2 +- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c > index d319c3020c09..84e9729badaa 100644 > --- a/fs/btrfs/check-integrity.c > +++ b/fs/btrfs/check-integrity.c > @@ -1601,7 +1601,7 @@ static int btrfsic_read_block(struct btrfsic_state *state, > BUG_ON(block_ctx->datav); > BUG_ON(block_ctx->pagev); > BUG_ON(block_ctx->mem_to_free); > - if (block_ctx->dev_bytenr & ((u64)PAGE_SIZE - 1)) { > + if (!PAGE_ALIGNED(block_ctx->dev_bytenr)) { > pr_info("btrfsic: read_block() with unaligned bytenr %llu\n", > block_ctx->dev_bytenr); > return -1; > @@ -1778,7 +1778,7 @@ static void btrfsic_process_written_block(struct btrfsic_dev_state *dev_state, > return; > } > is_metadata = 1; > - BUG_ON(BTRFS_SUPER_INFO_SIZE & (PAGE_SIZE - 1)); > + BUG_ON(!PAGE_ALIGNED(BTRFS_SUPER_INFO_SIZE)); > processed_len = BTRFS_SUPER_INFO_SIZE; > if (state->print_mask & > BTRFSIC_PRINT_MASK_TREE_BEFORE_SB_WRITE) { > @@ -2892,12 +2892,12 @@ int btrfsic_mount(struct btrfs_fs_info *fs_info, > struct list_head *dev_head = &fs_devices->devices; > struct btrfs_device *device; > > - if (fs_info->nodesize & ((u64)PAGE_SIZE - 1)) { > + if (!PAGE_ALIGNED(fs_info->nodesize)) { > pr_info("btrfsic: cannot handle nodesize %d not being a multiple of PAGE_SIZE %ld!\n", > fs_info->nodesize, PAGE_SIZE); > return -1; > } > - if (fs_info->sectorsize & ((u64)PAGE_SIZE - 1)) { > + if (!PAGE_ALIGNED(fs_info->sectorsize)) { > pr_info("btrfsic: cannot handle sectorsize %d not being a multiple of PAGE_SIZE %ld!\n", > fs_info->sectorsize, PAGE_SIZE); > return -1; > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index 2ab5591449f2..d5381f39a9e8 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -301,7 +301,7 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start, > blk_status_t ret; > int skip_sum = BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM; > > - WARN_ON(start & ((u64)PAGE_SIZE - 1)); > + WARN_ON(!PAGE_ALIGNED(start)); > cb = kmalloc(compressed_bio_size(fs_info, compressed_len), GFP_NOFS); > if (!cb) > return BLK_STS_RESOURCE; > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index bc0564c384de..5c52e91b01e8 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -2027,7 +2027,7 @@ int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end, > unsigned int extra_bits, > struct extent_state **cached_state, int dedupe) > { > - WARN_ON((end & (PAGE_SIZE - 1)) == 0); > + WARN_ON(PAGE_ALIGNED(end)); > return set_extent_delalloc(&BTRFS_I(inode)->io_tree, start, end, > extra_bits, cached_state); > } > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci 2018-12-05 14:23 [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED Johannes Thumshirn 2018-12-05 14:23 ` [PATCH 1/3] btrfs: use offset_in_page instead of open-coding it Johannes Thumshirn 2018-12-05 14:23 ` [PATCH 2/3] btrfs: use PAGE_ALIGNED " Johannes Thumshirn @ 2018-12-05 14:23 ` Johannes Thumshirn 2018-12-05 14:46 ` Julia Lawall 2018-12-06 14:54 ` [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED David Sterba 3 siblings, 1 reply; 10+ messages in thread From: Johannes Thumshirn @ 2018-12-05 14:23 UTC (permalink / raw) To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn, Julia Lawall Constructs like 'var & (PAGE_SIZE - 1)' or 'var & ~PAGE_MASK' can be replaced by the offset_in_page() macro instead of open-coding it. Add a coccinelle semantic patch to ease detection and conversion of these. This unfortunately doesn't account for the case when we want PAGE_ALIGNED() instead of offset_in_page() yet. Cc: Julia Lawall <julia.lawall@lip6.fr> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> --- scripts/coccinelle/api/offset_in_page.cocci | 81 +++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 scripts/coccinelle/api/offset_in_page.cocci diff --git a/scripts/coccinelle/api/offset_in_page.cocci b/scripts/coccinelle/api/offset_in_page.cocci new file mode 100644 index 000000000000..ea5b3a8e0390 --- /dev/null +++ b/scripts/coccinelle/api/offset_in_page.cocci @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0 +/// +/// Use offset_in_page macro on address instead of explicit computation. +/// +// Confidence: High +// Keywords: offset_in_page +// Comment: Based on vma_pages.cocci + +virtual context +virtual patch +virtual org +virtual report + + +//---------------------------------------------------------- +// For context mode +//---------------------------------------------------------- + +@r_context depends on context && !patch && !org && !report@ +expression E; +@@ + +( +* E & ~PAGE_MASK +| +* E & (PAGE_SIZE - 1) +) + + +//---------------------------------------------------------- +// For patch mode +//---------------------------------------------------------- + +@r_patch depends on !context && patch && !org && !report@ +expression E; +type T; +@@ + +( +- E & ~PAGE_MASK ++ offset_in_page(E) +| +- E & (PAGE_SIZE - 1) ++ offset_in_page(E) +| +- E & ((T)PAGE_SIZE - 1) ++ offset_in_page(E) +) + +//---------------------------------------------------------- +// For org mode +//---------------------------------------------------------- + +@r_org depends on !context && !patch && (org || report)@ +expression E; +position p; +@@ + + ( + * E@p & ~PAGE_MASK + | + * E@p & (PAGE_SIZE - 1) + ) + +@script:python depends on report@ +p << r_org.p; +x << r_org.E; +@@ + +msg="WARNING: Consider using offset_in_page helper on %s" % (x) +coccilib.report.print_report(p[0], msg) + +@script:python depends on org@ +p << r_org.p; +x << r_org.E; +@@ + +msg="WARNING: Consider using offset_in_page helper on %s" % (x) +msg_safe=msg.replace("[","@(").replace("]",")") +coccilib.org.print_todo(p[0], msg_safe) + -- 2.16.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci 2018-12-05 14:23 ` [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci Johannes Thumshirn @ 2018-12-05 14:46 ` Julia Lawall 2018-12-06 17:11 ` Johannes Thumshirn 0 siblings, 1 reply; 10+ messages in thread From: Julia Lawall @ 2018-12-05 14:46 UTC (permalink / raw) To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist, Masahiro Yamada On Wed, 5 Dec 2018, Johannes Thumshirn wrote: > Constructs like 'var & (PAGE_SIZE - 1)' or 'var & ~PAGE_MASK' can be > replaced by the offset_in_page() macro instead of open-coding it. > > Add a coccinelle semantic patch to ease detection and conversion of these. > > This unfortunately doesn't account for the case when we want PAGE_ALIGNED() > instead of offset_in_page() yet. > > Cc: Julia Lawall <julia.lawall@lip6.fr> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > --- > scripts/coccinelle/api/offset_in_page.cocci | 81 +++++++++++++++++++++++++++++ > 1 file changed, 81 insertions(+) > create mode 100644 scripts/coccinelle/api/offset_in_page.cocci > > diff --git a/scripts/coccinelle/api/offset_in_page.cocci b/scripts/coccinelle/api/offset_in_page.cocci > new file mode 100644 > index 000000000000..ea5b3a8e0390 > --- /dev/null > +++ b/scripts/coccinelle/api/offset_in_page.cocci > @@ -0,0 +1,81 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/// > +/// Use offset_in_page macro on address instead of explicit computation. > +/// > +// Confidence: High > +// Keywords: offset_in_page > +// Comment: Based on vma_pages.cocci > + > +virtual context > +virtual patch > +virtual org > +virtual report > + > + > +//---------------------------------------------------------- > +// For context mode > +//---------------------------------------------------------- > + > +@r_context depends on context && !patch && !org && !report@ > +expression E; > +@@ > + > +( > +* E & ~PAGE_MASK > +| > +* E & (PAGE_SIZE - 1) > +) > + > + > +//---------------------------------------------------------- > +// For patch mode > +//---------------------------------------------------------- > + > +@r_patch depends on !context && patch && !org && !report@ > +expression E; > +type T; > +@@ > + > +( > +- E & ~PAGE_MASK > ++ offset_in_page(E) > +| > +- E & (PAGE_SIZE - 1) > ++ offset_in_page(E) The two lines above should be subsumed by the two lines below. When there is a type metavariable that has no other dependencies, an isomorphism will consider that it is either present or absent. Why not include the cast case for the context and org cases? Masahiro will ultimately commit this. I have added him to CC. Thanks for the contribution. julia > +| > +- E & ((T)PAGE_SIZE - 1) > ++ offset_in_page(E) > +) > + > +//---------------------------------------------------------- > +// For org mode > +//---------------------------------------------------------- > + > +@r_org depends on !context && !patch && (org || report)@ > +expression E; > +position p; > +@@ > + > + ( > + * E@p & ~PAGE_MASK > + | > + * E@p & (PAGE_SIZE - 1) > + ) > + > +@script:python depends on report@ > +p << r_org.p; > +x << r_org.E; > +@@ > + > +msg="WARNING: Consider using offset_in_page helper on %s" % (x) > +coccilib.report.print_report(p[0], msg) > + > +@script:python depends on org@ > +p << r_org.p; > +x << r_org.E; > +@@ > + > +msg="WARNING: Consider using offset_in_page helper on %s" % (x) > +msg_safe=msg.replace("[","@(").replace("]",")") > +coccilib.org.print_todo(p[0], msg_safe) > + > -- > 2.16.4 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci 2018-12-05 14:46 ` Julia Lawall @ 2018-12-06 17:11 ` Johannes Thumshirn 2018-12-06 20:15 ` Julia Lawall 0 siblings, 1 reply; 10+ messages in thread From: Johannes Thumshirn @ 2018-12-06 17:11 UTC (permalink / raw) To: Julia Lawall; +Cc: David Sterba, Linux BTRFS Mailinglist, Masahiro Yamada On 05/12/2018 15:46, Julia Lawall wrote: [...] >> +@r_patch depends on !context && patch && !org && !report@ >> +expression E; >> +type T; >> +@@ >> + >> +( >> +- E & ~PAGE_MASK >> ++ offset_in_page(E) >> +| >> +- E & (PAGE_SIZE - 1) >> ++ offset_in_page(E) > > The two lines above should be subsumed by the two lines below. When there > is a type metavariable that has no other dependencies, an isomorphism will > consider that it is either present or absent. Oh OK, I'm sorry I'm not really into cocinelle so I guessed it might take some iterations. Do you have an example for this? > Why not include the cast case for the context and org cases? This is an oversight actually as I couldn't test it due to a bug in openSUSE's coccinelle rpm. Thanks, Johannes -- Johannes Thumshirn SUSE Labs Filesystems jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci 2018-12-06 17:11 ` Johannes Thumshirn @ 2018-12-06 20:15 ` Julia Lawall 0 siblings, 0 replies; 10+ messages in thread From: Julia Lawall @ 2018-12-06 20:15 UTC (permalink / raw) To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist, Masahiro Yamada On Thu, 6 Dec 2018, Johannes Thumshirn wrote: > On 05/12/2018 15:46, Julia Lawall wrote: > [...] > >> +@r_patch depends on !context && patch && !org && !report@ > >> +expression E; > >> +type T; > >> +@@ > >> + > >> +( > >> +- E & ~PAGE_MASK > >> ++ offset_in_page(E) > >> +| > >> +- E & (PAGE_SIZE - 1) > >> ++ offset_in_page(E) > > > > The two lines above should be subsumed by the two lines below. When there > > is a type metavariable that has no other dependencies, an isomorphism will > > consider that it is either present or absent. > > Oh OK, I'm sorry I'm not really into cocinelle so I guessed it might > take some iterations. > > Do you have an example for this? Expanation 1: Coccinelle as a file standard.iso that shows the isomorphisms (rewrite rules) that may be applied to semantic patches. One of the rules is: Expression @ not_ptr1 @ expression *X; @@ !X => X == NULL So if you have a pointer typed expression X and you write a transformation on !X, it will also apply to occurrences of X == NULL in the source code. In this way, you don't have to write so many variants. Likewise there is an isomorphism: Expression @ drop_cast @ expression E; pure type T; @@ (T)E => E That is, if you have a semantic patch with (T)X, then it will also apply to code that matches just X, without the cast. The word pure means that this isomorphism metavariable has to match a semantic patch term that is a metavariable and this metavariable can't be used elsewhere. If you wrote - (char)x Then you would probably not want that to apply without the (char) cast. But if you have just - (T)x for some randome unbound metavariable T, then perhaps you don't case about the cast to T. If you actually do, then you can put disable drop_cast in the header of your rule. Explanation 2: To see what your semantic patch is really doing, you can run spatch --parse-cocci sp.cocci Here is what I get for your patch rule, with some annotations added: @@ expression E; type T; @@ ( -E >>> offset_in_page(E) -& -~-PAGE_MASK | -~ >>> offset_in_page(E) -PAGE_MASK -& -E | // the following come from // - E & (PAGE_SIZE - 1) // + offset_in_page(E) -E // 1 >>> offset_in_page(E) -& -(-PAGE_SIZE -- -1-) | -E // 2 >>> offset_in_page(E) -& -PAGE_SIZE -- -1 | -( // 3 >>> offset_in_page(E) -PAGE_SIZE -- -1-) -& -E | -PAGE_SIZE // 4 >>> offset_in_page(E) -- -1 -& -E | // the following come from: // - E & ((T)PAGE_SIZE - 1) // + offset_in_page(E) -E >>> offset_in_page(E) -& -(-(-T -)-PAGE_SIZE -- -1-) | -E // same as 1 >>> offset_in_page(E) -& -(-PAGE_SIZE -- -1-) | -E >>> offset_in_page(E) -& -(-T -)-PAGE_SIZE -- -1 | -E // same as 2 >>> offset_in_page(E) -& -PAGE_SIZE -- -1 | -( >>> offset_in_page(E) -(-T -)-PAGE_SIZE -- -1-) -& -E | -( // same as 3 >>> offset_in_page(E) -PAGE_SIZE -- -1-) -& -E | -( >>> offset_in_page(E) -T -)-PAGE_SIZE -- -1 -& -E | -PAGE_SIZE // same as 4 >>> offset_in_page(E) -- -1 -& -E ) So all the transformation generated by - E & (PAGE_SIZE - 1) + offset_in_page(E) are also generated by - E & ((T)PAGE_SIZE - 1) + offset_in_page(E) I hope that is helpful. julia ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED 2018-12-05 14:23 [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED Johannes Thumshirn ` (2 preceding siblings ...) 2018-12-05 14:23 ` [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci Johannes Thumshirn @ 2018-12-06 14:54 ` David Sterba 3 siblings, 0 replies; 10+ messages in thread From: David Sterba @ 2018-12-06 14:54 UTC (permalink / raw) To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist On Wed, Dec 05, 2018 at 03:23:02PM +0100, Johannes Thumshirn wrote: > Use the offset_in_page() and PAGE_ALIGNED() macros instead of open-coding them > throughout btrfs. > > This series also includes a patch for 'make coccicheck' which is marked as an > RFC and I've CCed Julia in the hoping to get input from her. > > Johannes Thumshirn (3): > btrfs: use offset_in_page instead of open-coding it > btrfs: use PAGE_ALIGNED instead of open-coding it Added to misc-next, thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-12-06 20:15 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-05 14:23 [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED Johannes Thumshirn 2018-12-05 14:23 ` [PATCH 1/3] btrfs: use offset_in_page instead of open-coding it Johannes Thumshirn 2018-12-05 14:32 ` Nikolay Borisov 2018-12-05 14:23 ` [PATCH 2/3] btrfs: use PAGE_ALIGNED " Johannes Thumshirn 2018-12-05 14:31 ` Nikolay Borisov 2018-12-05 14:23 ` [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci Johannes Thumshirn 2018-12-05 14:46 ` Julia Lawall 2018-12-06 17:11 ` Johannes Thumshirn 2018-12-06 20:15 ` Julia Lawall 2018-12-06 14:54 ` [PATCH 0/3] btrfs: use offset_in_page and PAGE_ALIGNED David Sterba
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).