| Message ID | 36f81ee96c9868b1517dbbda553c14b917b5d5ee.1441131625.git.osandov@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 09/01/2015 03:01 PM, Omar Sandoval wrote: > From: Omar Sandoval <osandov@fb.com> > > These are going to be used for the free space tree bitmap items. > > Signed-off-by: Omar Sandoval <osandov@fb.com> Can we get sanity tests for these operations so we know they are properly unit tested? > --- > fs/btrfs/extent_io.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++ > fs/btrfs/extent_io.h | 6 +++ > 2 files changed, 107 insertions(+) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 02d05817cbdf..649e3b4eeb1b 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -5475,6 +5475,107 @@ void copy_extent_buffer(struct extent_buffer *dst, struct extent_buffer *src, > } > } > > +/* > + * The extent buffer bitmap operations are done with byte granularity because > + * bitmap items are not guaranteed to be aligned to a word and therefore a > + * single word in a bitmap may straddle two pages in the extent buffer. > + */ > +#define BIT_BYTE(nr) ((nr) / BITS_PER_BYTE) > +#define BYTE_MASK ((1 << BITS_PER_BYTE) - 1) > +#define BITMAP_FIRST_BYTE_MASK(start) \ > + ((BYTE_MASK << ((start) & (BITS_PER_BYTE - 1))) & BYTE_MASK) > +#define BITMAP_LAST_BYTE_MASK(nbits) \ > + (BYTE_MASK >> (-(nbits) & (BITS_PER_BYTE - 1))) > + > +int extent_buffer_test_bit(struct extent_buffer *eb, unsigned long start, > + unsigned long nr) > +{ > + size_t offset; > + char *kaddr; > + struct page *page; > + size_t byte_offset = BIT_BYTE(nr); > + size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); > + unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; > + > + offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); > + page = eb->pages[i]; > + WARN_ON(!PageUptodate(page)); > + kaddr = page_address(page); > + return 1U & (kaddr[offset] >> (nr & (BITS_PER_BYTE - 1))); > +} > + > +void extent_buffer_bitmap_set(struct extent_buffer *eb, unsigned long start, > + unsigned long pos, unsigned long len) > +{ > + size_t offset; > + char *kaddr; > + struct page *page; > + size_t byte_offset = BIT_BYTE(pos); > + size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); > + unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; > + const unsigned int size = pos + len; > + int bits_to_set = BITS_PER_BYTE - (pos % BITS_PER_BYTE); > + unsigned int mask_to_set = BITMAP_FIRST_BYTE_MASK(pos); > + > + offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); > + page = eb->pages[i]; > + WARN_ON(!PageUptodate(page)); > + kaddr = page_address(page); > + > + while (len >= bits_to_set) { > + kaddr[offset] |= mask_to_set; > + len -= bits_to_set; > + bits_to_set = BITS_PER_BYTE; > + mask_to_set = ~0U; > + if (++offset >= PAGE_CACHE_SIZE && len > 0) { > + offset = 0; > + page = eb->pages[++i]; > + WARN_ON(!PageUptodate(page)); > + kaddr = page_address(page); > + } > + } > + if (len) { > + mask_to_set &= BITMAP_LAST_BYTE_MASK(size); > + kaddr[offset] |= mask_to_set; > + } > +} > + > +void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long start, > + unsigned long pos, unsigned long len) > +{ > + size_t offset; > + char *kaddr; > + struct page *page; > + size_t byte_offset = BIT_BYTE(pos); > + size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); > + unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; > + const unsigned int size = pos + len; > + int bits_to_clear = BITS_PER_BYTE - (pos % BITS_PER_BYTE); > + unsigned int mask_to_clear = BITMAP_FIRST_BYTE_MASK(pos); > + > + offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); > + page = eb->pages[i]; > + WARN_ON(!PageUptodate(page)); > + kaddr = page_address(page); > + Abstract this offset finding logic to a helper function and then comment the hell out of it, I now have a migraine trying to figure out what is going on. 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
On Tue, Sep 01, 2015 at 03:25:54PM -0400, Josef Bacik wrote: > On 09/01/2015 03:01 PM, Omar Sandoval wrote: > >From: Omar Sandoval <osandov@fb.com> > > > >These are going to be used for the free space tree bitmap items. > > > >Signed-off-by: Omar Sandoval <osandov@fb.com> > > Can we get sanity tests for these operations so we know they are properly > unit tested? > No problem, I'll do that. > >--- > > fs/btrfs/extent_io.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > fs/btrfs/extent_io.h | 6 +++ > > 2 files changed, 107 insertions(+) > > > >diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > >index 02d05817cbdf..649e3b4eeb1b 100644 > >--- a/fs/btrfs/extent_io.c > >+++ b/fs/btrfs/extent_io.c > >@@ -5475,6 +5475,107 @@ void copy_extent_buffer(struct extent_buffer *dst, struct extent_buffer *src, > > } > > } > > > >+/* > >+ * The extent buffer bitmap operations are done with byte granularity because > >+ * bitmap items are not guaranteed to be aligned to a word and therefore a > >+ * single word in a bitmap may straddle two pages in the extent buffer. > >+ */ > >+#define BIT_BYTE(nr) ((nr) / BITS_PER_BYTE) > >+#define BYTE_MASK ((1 << BITS_PER_BYTE) - 1) > >+#define BITMAP_FIRST_BYTE_MASK(start) \ > >+ ((BYTE_MASK << ((start) & (BITS_PER_BYTE - 1))) & BYTE_MASK) > >+#define BITMAP_LAST_BYTE_MASK(nbits) \ > >+ (BYTE_MASK >> (-(nbits) & (BITS_PER_BYTE - 1))) > >+ > >+int extent_buffer_test_bit(struct extent_buffer *eb, unsigned long start, > >+ unsigned long nr) > >+{ > >+ size_t offset; > >+ char *kaddr; > >+ struct page *page; > >+ size_t byte_offset = BIT_BYTE(nr); > >+ size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); > >+ unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; > >+ > >+ offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); > >+ page = eb->pages[i]; > >+ WARN_ON(!PageUptodate(page)); > >+ kaddr = page_address(page); > >+ return 1U & (kaddr[offset] >> (nr & (BITS_PER_BYTE - 1))); > >+} > >+ > >+void extent_buffer_bitmap_set(struct extent_buffer *eb, unsigned long start, > >+ unsigned long pos, unsigned long len) > >+{ > >+ size_t offset; > >+ char *kaddr; > >+ struct page *page; > >+ size_t byte_offset = BIT_BYTE(pos); > >+ size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); > >+ unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; > >+ const unsigned int size = pos + len; > >+ int bits_to_set = BITS_PER_BYTE - (pos % BITS_PER_BYTE); > >+ unsigned int mask_to_set = BITMAP_FIRST_BYTE_MASK(pos); > >+ > >+ offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); > >+ page = eb->pages[i]; > >+ WARN_ON(!PageUptodate(page)); > >+ kaddr = page_address(page); > >+ > >+ while (len >= bits_to_set) { > >+ kaddr[offset] |= mask_to_set; > >+ len -= bits_to_set; > >+ bits_to_set = BITS_PER_BYTE; > >+ mask_to_set = ~0U; > >+ if (++offset >= PAGE_CACHE_SIZE && len > 0) { > >+ offset = 0; > >+ page = eb->pages[++i]; > >+ WARN_ON(!PageUptodate(page)); > >+ kaddr = page_address(page); > >+ } > >+ } > >+ if (len) { > >+ mask_to_set &= BITMAP_LAST_BYTE_MASK(size); > >+ kaddr[offset] |= mask_to_set; > >+ } > >+} > >+ > >+void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long start, > >+ unsigned long pos, unsigned long len) > >+{ > >+ size_t offset; > >+ char *kaddr; > >+ struct page *page; > >+ size_t byte_offset = BIT_BYTE(pos); > >+ size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); > >+ unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; > >+ const unsigned int size = pos + len; > >+ int bits_to_clear = BITS_PER_BYTE - (pos % BITS_PER_BYTE); > >+ unsigned int mask_to_clear = BITMAP_FIRST_BYTE_MASK(pos); > >+ > >+ offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); > >+ page = eb->pages[i]; > >+ WARN_ON(!PageUptodate(page)); > >+ kaddr = page_address(page); > >+ > > Abstract this offset finding logic to a helper function and then comment the > hell out of it, I now have a migraine trying to figure out what is going on. > Thanks, > > Josef Will do, thanks.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 02d05817cbdf..649e3b4eeb1b 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -5475,6 +5475,107 @@ void copy_extent_buffer(struct extent_buffer *dst, struct extent_buffer *src, } } +/* + * The extent buffer bitmap operations are done with byte granularity because + * bitmap items are not guaranteed to be aligned to a word and therefore a + * single word in a bitmap may straddle two pages in the extent buffer. + */ +#define BIT_BYTE(nr) ((nr) / BITS_PER_BYTE) +#define BYTE_MASK ((1 << BITS_PER_BYTE) - 1) +#define BITMAP_FIRST_BYTE_MASK(start) \ + ((BYTE_MASK << ((start) & (BITS_PER_BYTE - 1))) & BYTE_MASK) +#define BITMAP_LAST_BYTE_MASK(nbits) \ + (BYTE_MASK >> (-(nbits) & (BITS_PER_BYTE - 1))) + +int extent_buffer_test_bit(struct extent_buffer *eb, unsigned long start, + unsigned long nr) +{ + size_t offset; + char *kaddr; + struct page *page; + size_t byte_offset = BIT_BYTE(nr); + size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); + unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; + + offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); + page = eb->pages[i]; + WARN_ON(!PageUptodate(page)); + kaddr = page_address(page); + return 1U & (kaddr[offset] >> (nr & (BITS_PER_BYTE - 1))); +} + +void extent_buffer_bitmap_set(struct extent_buffer *eb, unsigned long start, + unsigned long pos, unsigned long len) +{ + size_t offset; + char *kaddr; + struct page *page; + size_t byte_offset = BIT_BYTE(pos); + size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); + unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; + const unsigned int size = pos + len; + int bits_to_set = BITS_PER_BYTE - (pos % BITS_PER_BYTE); + unsigned int mask_to_set = BITMAP_FIRST_BYTE_MASK(pos); + + offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); + page = eb->pages[i]; + WARN_ON(!PageUptodate(page)); + kaddr = page_address(page); + + while (len >= bits_to_set) { + kaddr[offset] |= mask_to_set; + len -= bits_to_set; + bits_to_set = BITS_PER_BYTE; + mask_to_set = ~0U; + if (++offset >= PAGE_CACHE_SIZE && len > 0) { + offset = 0; + page = eb->pages[++i]; + WARN_ON(!PageUptodate(page)); + kaddr = page_address(page); + } + } + if (len) { + mask_to_set &= BITMAP_LAST_BYTE_MASK(size); + kaddr[offset] |= mask_to_set; + } +} + +void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long start, + unsigned long pos, unsigned long len) +{ + size_t offset; + char *kaddr; + struct page *page; + size_t byte_offset = BIT_BYTE(pos); + size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1); + unsigned long i = (start_offset + start + byte_offset) >> PAGE_CACHE_SHIFT; + const unsigned int size = pos + len; + int bits_to_clear = BITS_PER_BYTE - (pos % BITS_PER_BYTE); + unsigned int mask_to_clear = BITMAP_FIRST_BYTE_MASK(pos); + + offset = (start_offset + start + byte_offset) & (PAGE_CACHE_SIZE - 1); + page = eb->pages[i]; + WARN_ON(!PageUptodate(page)); + kaddr = page_address(page); + + while (len >= bits_to_clear) { + kaddr[offset] &= ~mask_to_clear; + len -= bits_to_clear; + bits_to_clear = BITS_PER_BYTE; + mask_to_clear = ~0U; + if (++offset >= PAGE_CACHE_SIZE && len > 0) { + offset = 0; + page = eb->pages[++i]; + WARN_ON(!PageUptodate(page)); + kaddr = page_address(page); + } + } + if (len) { + mask_to_clear &= BITMAP_LAST_BYTE_MASK(size); + kaddr[offset] &= ~mask_to_clear; + } +} + static inline bool areas_overlap(unsigned long src, unsigned long dst, unsigned long len) { unsigned long distance = (src > dst) ? src - dst : dst - src; diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index c668f36898d3..9185a20081d7 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -309,6 +309,12 @@ void memmove_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset, unsigned long src_offset, unsigned long len); void memset_extent_buffer(struct extent_buffer *eb, char c, unsigned long start, unsigned long len); +int extent_buffer_test_bit(struct extent_buffer *eb, unsigned long start, + unsigned long pos); +void extent_buffer_bitmap_set(struct extent_buffer *eb, unsigned long start, + unsigned long pos, unsigned long len); +void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long start, + unsigned long pos, unsigned long len); void clear_extent_buffer_dirty(struct extent_buffer *eb); int set_extent_buffer_dirty(struct extent_buffer *eb); int set_extent_buffer_uptodate(struct extent_buffer *eb);