* don't poke into bio internals V2 @ 2016-11-25 8:07 Christoph Hellwig 2016-11-25 8:07 ` [PATCH 1/8] btrfs: use bio iterators for the decompression handlers Christoph Hellwig ` (8 more replies) 0 siblings, 9 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval Hi all, this series has a few patches that switch btrfs to use the proper helpers for accessing bio internals. This helps to prepare for supporting multi-page bio_vecs, which are currently under development. Changes since v1: - fixed two compression related bugs - various minor cleanups - dropped the last patch of the old series for now - I think the old code there is already buggy, but it seems like no one can fully explain to me what it's actually supposed to do.. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/8] btrfs: use bio iterators for the decompression handlers 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig @ 2016-11-25 8:07 ` Christoph Hellwig 2016-11-25 8:07 ` [PATCH 2/8] btrfs: don't access the bio directly in the raid5/6 code Christoph Hellwig ` (7 subsequent siblings) 8 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval Pass the full bio to the decompression routines and use bio iterators to iterate over the data in the bio. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/compression.c | 123 +++++++++++++++++-------------------------------- fs/btrfs/compression.h | 12 ++--- fs/btrfs/lzo.c | 17 ++----- fs/btrfs/zlib.c | 15 ++---- 4 files changed, 55 insertions(+), 112 deletions(-) diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index d4d8b7e..fcaa25f 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -81,9 +81,9 @@ struct compressed_bio { u32 sums; }; -static int btrfs_decompress_biovec(int type, struct page **pages_in, - u64 disk_start, struct bio_vec *bvec, - int vcnt, size_t srclen); +static int btrfs_decompress_bio(int type, struct page **pages_in, + u64 disk_start, struct bio *orig_bio, + size_t srclen); static inline int compressed_bio_size(struct btrfs_root *root, unsigned long disk_size) @@ -175,11 +175,10 @@ static void end_compressed_bio_read(struct bio *bio) /* ok, we're the last bio for this extent, lets start * the decompression. */ - ret = btrfs_decompress_biovec(cb->compress_type, + ret = btrfs_decompress_bio(cb->compress_type, cb->compressed_pages, cb->start, - cb->orig_bio->bi_io_vec, - cb->orig_bio->bi_vcnt, + cb->orig_bio, cb->compressed_len); csum_failed: if (ret) @@ -959,9 +958,7 @@ int btrfs_compress_pages(int type, struct address_space *mapping, * * disk_start is the starting logical offset of this array in the file * - * bvec is a bio_vec of pages from the file that we want to decompress into - * - * vcnt is the count of pages in the biovec + * orig_bio contains the pages from the file that we want to decompress into * * srclen is the number of bytes in pages_in * @@ -970,18 +967,18 @@ int btrfs_compress_pages(int type, struct address_space *mapping, * be contiguous. They all correspond to the range of bytes covered by * the compressed extent. */ -static int btrfs_decompress_biovec(int type, struct page **pages_in, - u64 disk_start, struct bio_vec *bvec, - int vcnt, size_t srclen) +static int btrfs_decompress_bio(int type, struct page **pages_in, + u64 disk_start, struct bio *orig_bio, + size_t srclen) { struct list_head *workspace; int ret; workspace = find_workspace(type); - ret = btrfs_compress_op[type-1]->decompress_biovec(workspace, pages_in, - disk_start, - bvec, vcnt, srclen); + ret = btrfs_compress_op[type-1]->decompress_bio(workspace, pages_in, + disk_start, orig_bio, + srclen); free_workspace(type, workspace); return ret; } @@ -1021,9 +1018,7 @@ void btrfs_exit_compress(void) */ int btrfs_decompress_buf2page(char *buf, unsigned long buf_start, unsigned long total_out, u64 disk_start, - struct bio_vec *bvec, int vcnt, - unsigned long *pg_index, - unsigned long *pg_offset) + struct bio *bio) { unsigned long buf_offset; unsigned long current_buf_start; @@ -1031,13 +1026,13 @@ int btrfs_decompress_buf2page(char *buf, unsigned long buf_start, unsigned long working_bytes = total_out - buf_start; unsigned long bytes; char *kaddr; - struct page *page_out = bvec[*pg_index].bv_page; + struct bio_vec bvec = bio_iter_iovec(bio, bio->bi_iter); /* * start byte is the first byte of the page we're currently * copying into relative to the start of the compressed data. */ - start_byte = page_offset(page_out) - disk_start; + start_byte = page_offset(bvec.bv_page) - disk_start; /* we haven't yet hit data corresponding to this page */ if (total_out <= start_byte) @@ -1057,80 +1052,46 @@ int btrfs_decompress_buf2page(char *buf, unsigned long buf_start, /* copy bytes from the working buffer into the pages */ while (working_bytes > 0) { - bytes = min(PAGE_SIZE - *pg_offset, - PAGE_SIZE - buf_offset); + bytes = min_t(unsigned long, bvec.bv_len, + PAGE_SIZE - buf_offset); bytes = min(bytes, working_bytes); - kaddr = kmap_atomic(page_out); - memcpy(kaddr + *pg_offset, buf + buf_offset, bytes); + + kaddr = kmap_atomic(bvec.bv_page); + memcpy(kaddr + bvec.bv_offset, buf + buf_offset, bytes); kunmap_atomic(kaddr); - flush_dcache_page(page_out); + flush_dcache_page(bvec.bv_page); - *pg_offset += bytes; buf_offset += bytes; working_bytes -= bytes; current_buf_start += bytes; /* check if we need to pick another page */ - if (*pg_offset == PAGE_SIZE) { - (*pg_index)++; - if (*pg_index >= vcnt) - return 0; + bio_advance(bio, bytes); + if (!bio->bi_iter.bi_size) + return 0; + bvec = bio_iter_iovec(bio, bio->bi_iter); - page_out = bvec[*pg_index].bv_page; - *pg_offset = 0; - start_byte = page_offset(page_out) - disk_start; + start_byte = page_offset(bvec.bv_page) - disk_start; - /* - * make sure our new page is covered by this - * working buffer - */ - if (total_out <= start_byte) - return 1; + /* + * make sure our new page is covered by this + * working buffer + */ + if (total_out <= start_byte) + return 1; - /* - * the next page in the biovec might not be adjacent - * to the last page, but it might still be found - * inside this working buffer. bump our offset pointer - */ - if (total_out > start_byte && - current_buf_start < start_byte) { - buf_offset = start_byte - buf_start; - working_bytes = total_out - start_byte; - current_buf_start = buf_start + buf_offset; - } + /* + * the next page in the biovec might not be adjacent + * to the last page, but it might still be found + * inside this working buffer. bump our offset pointer + */ + if (total_out > start_byte && + current_buf_start < start_byte) { + buf_offset = start_byte - buf_start; + working_bytes = total_out - start_byte; + current_buf_start = buf_start + buf_offset; } } return 1; } - -/* - * When uncompressing data, we need to make sure and zero any parts of - * the biovec that were not filled in by the decompression code. pg_index - * and pg_offset indicate the last page and the last offset of that page - * that have been filled in. This will zero everything remaining in the - * biovec. - */ -void btrfs_clear_biovec_end(struct bio_vec *bvec, int vcnt, - unsigned long pg_index, - unsigned long pg_offset) -{ - while (pg_index < vcnt) { - struct page *page = bvec[pg_index].bv_page; - unsigned long off = bvec[pg_index].bv_offset; - unsigned long len = bvec[pg_index].bv_len; - - if (pg_offset < off) - pg_offset = off; - if (pg_offset < off + len) { - unsigned long bytes = off + len - pg_offset; - char *kaddr; - - kaddr = kmap_atomic(page); - memset(kaddr + pg_offset, 0, bytes); - kunmap_atomic(kaddr); - } - pg_index++; - pg_offset = 0; - } -} diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h index f49d8b8..0987957 100644 --- a/fs/btrfs/compression.h +++ b/fs/btrfs/compression.h @@ -34,9 +34,7 @@ int btrfs_decompress(int type, unsigned char *data_in, struct page *dest_page, unsigned long start_byte, size_t srclen, size_t destlen); int btrfs_decompress_buf2page(char *buf, unsigned long buf_start, unsigned long total_out, u64 disk_start, - struct bio_vec *bvec, int vcnt, - unsigned long *pg_index, - unsigned long *pg_offset); + struct bio *bio); int btrfs_submit_compressed_write(struct inode *inode, u64 start, unsigned long len, u64 disk_start, @@ -45,9 +43,6 @@ int btrfs_submit_compressed_write(struct inode *inode, u64 start, unsigned long nr_pages); int btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, int mirror_num, unsigned long bio_flags); -void btrfs_clear_biovec_end(struct bio_vec *bvec, int vcnt, - unsigned long pg_index, - unsigned long pg_offset); enum btrfs_compression_type { BTRFS_COMPRESS_NONE = 0, @@ -72,11 +67,10 @@ struct btrfs_compress_op { unsigned long *total_out, unsigned long max_out); - int (*decompress_biovec)(struct list_head *workspace, + int (*decompress_bio)(struct list_head *workspace, struct page **pages_in, u64 disk_start, - struct bio_vec *bvec, - int vcnt, + struct bio *orig_bio, size_t srclen); int (*decompress)(struct list_head *workspace, diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c index 48655da..45d2698 100644 --- a/fs/btrfs/lzo.c +++ b/fs/btrfs/lzo.c @@ -254,25 +254,21 @@ static int lzo_compress_pages(struct list_head *ws, return ret; } -static int lzo_decompress_biovec(struct list_head *ws, +static int lzo_decompress_bio(struct list_head *ws, struct page **pages_in, u64 disk_start, - struct bio_vec *bvec, - int vcnt, + struct bio *orig_bio, size_t srclen) { struct workspace *workspace = list_entry(ws, struct workspace, list); int ret = 0, ret2; char *data_in; unsigned long page_in_index = 0; - unsigned long page_out_index = 0; unsigned long total_pages_in = DIV_ROUND_UP(srclen, PAGE_SIZE); unsigned long buf_start; unsigned long buf_offset = 0; unsigned long bytes; unsigned long working_bytes; - unsigned long pg_offset; - size_t in_len; size_t out_len; unsigned long in_offset; @@ -292,7 +288,6 @@ static int lzo_decompress_biovec(struct list_head *ws, in_page_bytes_left = PAGE_SIZE - LZO_LEN; tot_out = 0; - pg_offset = 0; while (tot_in < tot_len) { in_len = read_compress_length(data_in + in_offset); @@ -365,16 +360,14 @@ static int lzo_decompress_biovec(struct list_head *ws, tot_out += out_len; ret2 = btrfs_decompress_buf2page(workspace->buf, buf_start, - tot_out, disk_start, - bvec, vcnt, - &page_out_index, &pg_offset); + tot_out, disk_start, orig_bio); if (ret2 == 0) break; } done: kunmap(pages_in[page_in_index]); if (!ret) - btrfs_clear_biovec_end(bvec, vcnt, page_out_index, pg_offset); + zero_fill_bio(orig_bio); return ret; } @@ -438,6 +431,6 @@ const struct btrfs_compress_op btrfs_lzo_compress = { .alloc_workspace = lzo_alloc_workspace, .free_workspace = lzo_free_workspace, .compress_pages = lzo_compress_pages, - .decompress_biovec = lzo_decompress_biovec, + .decompress_bio = lzo_decompress_bio, .decompress = lzo_decompress, }; diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c index 441b81a..0d5f28e 100644 --- a/fs/btrfs/zlib.c +++ b/fs/btrfs/zlib.c @@ -210,10 +210,9 @@ static int zlib_compress_pages(struct list_head *ws, return ret; } -static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in, +static int zlib_decompress_bio(struct list_head *ws, struct page **pages_in, u64 disk_start, - struct bio_vec *bvec, - int vcnt, + struct bio *orig_bio, size_t srclen) { struct workspace *workspace = list_entry(ws, struct workspace, list); @@ -222,10 +221,8 @@ static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in, char *data_in; size_t total_out = 0; unsigned long page_in_index = 0; - unsigned long page_out_index = 0; unsigned long total_pages_in = DIV_ROUND_UP(srclen, PAGE_SIZE); unsigned long buf_start; - unsigned long pg_offset; data_in = kmap(pages_in[page_in_index]); workspace->strm.next_in = data_in; @@ -235,7 +232,6 @@ static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in, workspace->strm.total_out = 0; workspace->strm.next_out = workspace->buf; workspace->strm.avail_out = PAGE_SIZE; - pg_offset = 0; /* If it's deflate, and it's got no preset dictionary, then we can tell zlib to skip the adler32 check. */ @@ -266,8 +262,7 @@ static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in, ret2 = btrfs_decompress_buf2page(workspace->buf, buf_start, total_out, disk_start, - bvec, vcnt, - &page_out_index, &pg_offset); + orig_bio); if (ret2 == 0) { ret = 0; goto done; @@ -300,7 +295,7 @@ static int zlib_decompress_biovec(struct list_head *ws, struct page **pages_in, if (data_in) kunmap(pages_in[page_in_index]); if (!ret) - btrfs_clear_biovec_end(bvec, vcnt, page_out_index, pg_offset); + zero_fill_bio(orig_bio); return ret; } @@ -407,6 +402,6 @@ const struct btrfs_compress_op btrfs_zlib_compress = { .alloc_workspace = zlib_alloc_workspace, .free_workspace = zlib_free_workspace, .compress_pages = zlib_compress_pages, - .decompress_biovec = zlib_decompress_biovec, + .decompress_bio = zlib_decompress_bio, .decompress = zlib_decompress, }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/8] btrfs: don't access the bio directly in the raid5/6 code 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig 2016-11-25 8:07 ` [PATCH 1/8] btrfs: use bio iterators for the decompression handlers Christoph Hellwig @ 2016-11-25 8:07 ` Christoph Hellwig 2016-11-25 8:07 ` [PATCH 3/8] btrfs: don't access the bio directly in the direct I/O code Christoph Hellwig ` (6 subsequent siblings) 8 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval Just use bio_for_each_segment_all to iterate over all segments. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Omar Sandoval <osandov@fb.com> --- fs/btrfs/raid56.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c index d016d4a..eece126 100644 --- a/fs/btrfs/raid56.c +++ b/fs/btrfs/raid56.c @@ -1144,10 +1144,10 @@ static void validate_rbio_for_rmw(struct btrfs_raid_bio *rbio) static void index_rbio_pages(struct btrfs_raid_bio *rbio) { struct bio *bio; + struct bio_vec *bvec; u64 start; unsigned long stripe_offset; unsigned long page_index; - struct page *p; int i; spin_lock_irq(&rbio->bio_list_lock); @@ -1156,10 +1156,8 @@ static void index_rbio_pages(struct btrfs_raid_bio *rbio) stripe_offset = start - rbio->bbio->raid_map[0]; page_index = stripe_offset >> PAGE_SHIFT; - for (i = 0; i < bio->bi_vcnt; i++) { - p = bio->bi_io_vec[i].bv_page; - rbio->bio_pages[page_index + i] = p; - } + bio_for_each_segment_all(bvec, bio, i) + rbio->bio_pages[page_index + i] = bvec->bv_page; } spin_unlock_irq(&rbio->bio_list_lock); } @@ -1433,13 +1431,11 @@ static int fail_bio_stripe(struct btrfs_raid_bio *rbio, */ static void set_bio_pages_uptodate(struct bio *bio) { + struct bio_vec *bvec; int i; - struct page *p; - for (i = 0; i < bio->bi_vcnt; i++) { - p = bio->bi_io_vec[i].bv_page; - SetPageUptodate(p); - } + bio_for_each_segment_all(bvec, bio, i) + SetPageUptodate(bvec->bv_page); } /* -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/8] btrfs: don't access the bio directly in the direct I/O code 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig 2016-11-25 8:07 ` [PATCH 1/8] btrfs: use bio iterators for the decompression handlers Christoph Hellwig 2016-11-25 8:07 ` [PATCH 2/8] btrfs: don't access the bio directly in the raid5/6 code Christoph Hellwig @ 2016-11-25 8:07 ` Christoph Hellwig 2016-11-25 8:07 ` [PATCH 4/8] btrfs: don't access the bio directly in btrfs_csum_one_bio Christoph Hellwig ` (5 subsequent siblings) 8 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval Just use bio_for_each_segment_all to iterate over all segments. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Omar Sandoval <osandov@fb.com> --- fs/btrfs/inode.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 147df4c..3f09cb6 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -8394,7 +8394,7 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip, struct btrfs_root *root = BTRFS_I(inode)->root; struct bio *bio; struct bio *orig_bio = dip->orig_bio; - struct bio_vec *bvec = orig_bio->bi_io_vec; + struct bio_vec *bvec; u64 start_sector = orig_bio->bi_iter.bi_sector; u64 file_offset = dip->logical_offset; u64 submit_len = 0; @@ -8403,7 +8403,7 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip, int async_submit = 0; int nr_sectors; int ret; - int i; + int i, j; map_length = orig_bio->bi_iter.bi_size; ret = btrfs_map_block(root->fs_info, btrfs_op(orig_bio), @@ -8433,7 +8433,7 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip, btrfs_io_bio(bio)->logical = file_offset; atomic_inc(&dip->pending_bios); - while (bvec <= (orig_bio->bi_io_vec + orig_bio->bi_vcnt - 1)) { + bio_for_each_segment_all(bvec, orig_bio, j) { nr_sectors = BTRFS_BYTES_TO_BLKS(root->fs_info, bvec->bv_len); i = 0; next_block: @@ -8487,7 +8487,6 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip, i++; goto next_block; } - bvec++; } } -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/8] btrfs: don't access the bio directly in btrfs_csum_one_bio 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig ` (2 preceding siblings ...) 2016-11-25 8:07 ` [PATCH 3/8] btrfs: don't access the bio directly in the direct I/O code Christoph Hellwig @ 2016-11-25 8:07 ` Christoph Hellwig 2016-11-25 8:07 ` [PATCH 5/8] btrfs: use bi_size Christoph Hellwig ` (4 subsequent siblings) 8 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval Use bio_for_each_segment_all to iterate over the segments instead. This requires a bit of reshuffling so that we only lookup up the ordered item once inside the bio_for_each_segment_all loop. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Omar Sandoval <osandov@fb.com> --- fs/btrfs/file-item.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c index d0d571c..fa8aa53 100644 --- a/fs/btrfs/file-item.c +++ b/fs/btrfs/file-item.c @@ -447,13 +447,12 @@ int btrfs_csum_one_bio(struct btrfs_root *root, struct inode *inode, struct bio *bio, u64 file_start, int contig) { struct btrfs_ordered_sum *sums; - struct btrfs_ordered_extent *ordered; + struct btrfs_ordered_extent *ordered = NULL; char *data; - struct bio_vec *bvec = bio->bi_io_vec; - int bio_index = 0; + struct bio_vec *bvec; int index; int nr_sectors; - int i; + int i, j; unsigned long total_bytes = 0; unsigned long this_sum_bytes = 0; u64 offset; @@ -470,17 +469,20 @@ int btrfs_csum_one_bio(struct btrfs_root *root, struct inode *inode, if (contig) offset = file_start; else - offset = page_offset(bvec->bv_page) + bvec->bv_offset; + offset = 0; /* shut up gcc */ - ordered = btrfs_lookup_ordered_extent(inode, offset); - BUG_ON(!ordered); /* Logic error */ sums->bytenr = (u64)bio->bi_iter.bi_sector << 9; index = 0; - while (bio_index < bio->bi_vcnt) { + bio_for_each_segment_all(bvec, bio, j) { if (!contig) offset = page_offset(bvec->bv_page) + bvec->bv_offset; + if (!ordered) { + ordered = btrfs_lookup_ordered_extent(inode, offset); + BUG_ON(!ordered); /* Logic error */ + } + data = kmap_atomic(bvec->bv_page); nr_sectors = BTRFS_BYTES_TO_BLKS(root->fs_info, @@ -529,9 +531,6 @@ int btrfs_csum_one_bio(struct btrfs_root *root, struct inode *inode, } kunmap_atomic(data); - - bio_index++; - bvec++; } this_sum_bytes = 0; btrfs_add_ordered_sum(inode, ordered, sums); -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/8] btrfs: use bi_size 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig ` (3 preceding siblings ...) 2016-11-25 8:07 ` [PATCH 4/8] btrfs: don't access the bio directly in btrfs_csum_one_bio Christoph Hellwig @ 2016-11-25 8:07 ` Christoph Hellwig 2016-11-25 8:07 ` [PATCH 6/8] btrfs: calculate end of bio offset properly Christoph Hellwig ` (3 subsequent siblings) 8 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval Instead of using bi_vcnt to calculate it. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Omar Sandoval <osandov@fb.com> --- fs/btrfs/compression.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index fcaa25f..77042a8 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -562,7 +562,6 @@ static noinline int add_ra_bio_pages(struct inode *inode, * * bio->bi_iter.bi_sector points to the compressed extent on disk * bio->bi_io_vec points to all of the inode pages - * bio->bi_vcnt is a count of pages * * After the compressed pages are read, we copy the bytes into the * bio we were passed and then call the bio end_io calls @@ -574,7 +573,6 @@ int btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, struct extent_map_tree *em_tree; struct compressed_bio *cb; struct btrfs_root *root = BTRFS_I(inode)->root; - unsigned long uncompressed_len = bio->bi_vcnt * PAGE_SIZE; unsigned long compressed_len; unsigned long nr_pages; unsigned long pg_index; @@ -619,7 +617,7 @@ int btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, free_extent_map(em); em = NULL; - cb->len = uncompressed_len; + cb->len = bio->bi_iter.bi_size; cb->compressed_len = compressed_len; cb->compress_type = extent_compress_type(bio_flags); cb->orig_bio = bio; @@ -647,8 +645,7 @@ int btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, add_ra_bio_pages(inode, em_start + em_len, cb); /* include any pages we added in add_ra-bio_pages */ - uncompressed_len = bio->bi_vcnt * PAGE_SIZE; - cb->len = uncompressed_len; + cb->len = bio->bi_iter.bi_size; comp_bio = compressed_bio_alloc(bdev, cur_disk_byte, GFP_NOFS); if (!comp_bio) -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/8] btrfs: calculate end of bio offset properly 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig ` (4 preceding siblings ...) 2016-11-25 8:07 ` [PATCH 5/8] btrfs: use bi_size Christoph Hellwig @ 2016-11-25 8:07 ` Christoph Hellwig 2016-11-28 21:38 ` Omar Sandoval 2016-11-25 8:07 ` [PATCH 7/8] btrfs: refactor __btrfs_lookup_bio_sums to use bio_for_each_segment_all Christoph Hellwig ` (2 subsequent siblings) 8 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval Use the bvec offset and len members to prepare for multipage bvecs. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/compression.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index 77042a8..3c1c25c 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -445,6 +445,13 @@ int btrfs_submit_compressed_write(struct inode *inode, u64 start, return 0; } +static u64 bio_end_offset(struct bio *bio) +{ + struct bio_vec *last = &bio->bi_io_vec[bio->bi_vcnt - 1]; + + return page_offset(last->bv_page) + last->bv_len + last->bv_offset; +} + static noinline int add_ra_bio_pages(struct inode *inode, u64 compressed_end, struct compressed_bio *cb) @@ -463,8 +470,7 @@ static noinline int add_ra_bio_pages(struct inode *inode, u64 end; int misses = 0; - page = cb->orig_bio->bi_io_vec[cb->orig_bio->bi_vcnt - 1].bv_page; - last_offset = (page_offset(page) + PAGE_SIZE); + last_offset = bio_end_offset(cb->orig_bio); em_tree = &BTRFS_I(inode)->extent_tree; tree = &BTRFS_I(inode)->io_tree; -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 6/8] btrfs: calculate end of bio offset properly 2016-11-25 8:07 ` [PATCH 6/8] btrfs: calculate end of bio offset properly Christoph Hellwig @ 2016-11-28 21:38 ` Omar Sandoval 0 siblings, 0 replies; 11+ messages in thread From: Omar Sandoval @ 2016-11-28 21:38 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-btrfs, Liu Bo On Fri, Nov 25, 2016 at 09:07:51AM +0100, Christoph Hellwig wrote: > Use the bvec offset and len members to prepare for multipage bvecs. Reviewed-by: Omar Sandoval <osandov@fb.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/btrfs/compression.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index 77042a8..3c1c25c 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -445,6 +445,13 @@ int btrfs_submit_compressed_write(struct inode *inode, u64 start, > return 0; > } > > +static u64 bio_end_offset(struct bio *bio) > +{ > + struct bio_vec *last = &bio->bi_io_vec[bio->bi_vcnt - 1]; > + > + return page_offset(last->bv_page) + last->bv_len + last->bv_offset; > +} > + > static noinline int add_ra_bio_pages(struct inode *inode, > u64 compressed_end, > struct compressed_bio *cb) > @@ -463,8 +470,7 @@ static noinline int add_ra_bio_pages(struct inode *inode, > u64 end; > int misses = 0; > > - page = cb->orig_bio->bi_io_vec[cb->orig_bio->bi_vcnt - 1].bv_page; > - last_offset = (page_offset(page) + PAGE_SIZE); > + last_offset = bio_end_offset(cb->orig_bio); > em_tree = &BTRFS_I(inode)->extent_tree; > tree = &BTRFS_I(inode)->io_tree; > > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 7/8] btrfs: refactor __btrfs_lookup_bio_sums to use bio_for_each_segment_all 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig ` (5 preceding siblings ...) 2016-11-25 8:07 ` [PATCH 6/8] btrfs: calculate end of bio offset properly Christoph Hellwig @ 2016-11-25 8:07 ` Christoph Hellwig 2016-11-25 8:07 ` [PATCH 8/8] btrfs: use bio_for_each_segment_all in __btrfsic_submit_bio Christoph Hellwig 2016-11-25 9:40 ` don't poke into bio internals V2 David Sterba 8 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval Rework the loop a little bit to use the generic bio_for_each_segment_all helper for iterating over the bio. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Omar Sandoval <osandov@fb.com> --- fs/btrfs/file-item.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c index fa8aa53..6b8fda3 100644 --- a/fs/btrfs/file-item.c +++ b/fs/btrfs/file-item.c @@ -163,7 +163,7 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root *root, struct inode *inode, struct bio *bio, u64 logical_offset, u32 *dst, int dio) { - struct bio_vec *bvec = bio->bi_io_vec; + struct bio_vec *bvec; struct btrfs_io_bio *btrfs_bio = btrfs_io_bio(bio); struct btrfs_csum_item *item = NULL; struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree; @@ -176,8 +176,7 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root *root, u64 page_bytes_left; u32 diff; int nblocks; - int bio_index = 0; - int count; + int count = 0, i; u16 csum_size = btrfs_super_csum_size(root->fs_info->super_copy); path = btrfs_alloc_path(); @@ -223,8 +222,11 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root *root, if (dio) offset = logical_offset; - page_bytes_left = bvec->bv_len; - while (bio_index < bio->bi_vcnt) { + bio_for_each_segment_all(bvec, bio, i) { + page_bytes_left = bvec->bv_len; + if (count) + goto next; + if (!dio) offset = page_offset(bvec->bv_page) + bvec->bv_offset; count = btrfs_find_ordered_sum(inode, offset, disk_bytenr, @@ -285,29 +287,17 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root *root, found: csum += count * csum_size; nblocks -= count; - +next: while (count--) { disk_bytenr += root->sectorsize; offset += root->sectorsize; page_bytes_left -= root->sectorsize; - if (!page_bytes_left) { - bio_index++; - /* - * make sure we're still inside the - * bio before we update page_bytes_left - */ - if (bio_index >= bio->bi_vcnt) { - WARN_ON_ONCE(count); - goto done; - } - bvec++; - page_bytes_left = bvec->bv_len; - } - + if (!page_bytes_left) + break; /* move to next bio */ } } -done: + WARN_ON_ONCE(count); btrfs_free_path(path); return 0; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 8/8] btrfs: use bio_for_each_segment_all in __btrfsic_submit_bio 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig ` (6 preceding siblings ...) 2016-11-25 8:07 ` [PATCH 7/8] btrfs: refactor __btrfs_lookup_bio_sums to use bio_for_each_segment_all Christoph Hellwig @ 2016-11-25 8:07 ` Christoph Hellwig 2016-11-25 9:40 ` don't poke into bio internals V2 David Sterba 8 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-11-25 8:07 UTC (permalink / raw) To: linux-btrfs; +Cc: Liu Bo, Omar Sandoval And remove the bogus check for a NULL return value from kmap, which can't happen. While we're at it: I don't think that kmapping up to 256 will work without deadlocks on highmem machines, a better idea would be to use vm_map_ram to map all of them into a single virtual address range. Incidentally that would also simplify the code a lot. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Omar Sandoval <osandov@fb.com> --- fs/btrfs/check-integrity.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c index a6f657f..86f681f 100644 --- a/fs/btrfs/check-integrity.c +++ b/fs/btrfs/check-integrity.c @@ -2819,10 +2819,11 @@ static void __btrfsic_submit_bio(struct bio *bio) * btrfsic_mount(), this might return NULL */ dev_state = btrfsic_dev_state_lookup(bio->bi_bdev); if (NULL != dev_state && - (bio_op(bio) == REQ_OP_WRITE) && NULL != bio->bi_io_vec) { + (bio_op(bio) == REQ_OP_WRITE) && bio_has_data(bio)) { unsigned int i; u64 dev_bytenr; u64 cur_bytenr; + struct bio_vec *bvec; int bio_is_patched; char **mapped_datav; @@ -2840,32 +2841,23 @@ static void __btrfsic_submit_bio(struct bio *bio) if (!mapped_datav) goto leave; cur_bytenr = dev_bytenr; - for (i = 0; i < bio->bi_vcnt; i++) { - BUG_ON(bio->bi_io_vec[i].bv_len != PAGE_SIZE); - mapped_datav[i] = kmap(bio->bi_io_vec[i].bv_page); - if (!mapped_datav[i]) { - while (i > 0) { - i--; - kunmap(bio->bi_io_vec[i].bv_page); - } - kfree(mapped_datav); - goto leave; - } + + bio_for_each_segment_all(bvec, bio, i) { + BUG_ON(bvec->bv_len != PAGE_SIZE); + mapped_datav[i] = kmap(bvec->bv_page); + if (dev_state->state->print_mask & BTRFSIC_PRINT_MASK_SUBMIT_BIO_BH_VERBOSE) pr_info("#%u: bytenr=%llu, len=%u, offset=%u\n", - i, cur_bytenr, bio->bi_io_vec[i].bv_len, - bio->bi_io_vec[i].bv_offset); - cur_bytenr += bio->bi_io_vec[i].bv_len; + i, cur_bytenr, bvec->bv_len, bvec->bv_offset); + cur_bytenr += bvec->bv_len; } btrfsic_process_written_block(dev_state, dev_bytenr, mapped_datav, bio->bi_vcnt, bio, &bio_is_patched, NULL, bio->bi_opf); - while (i > 0) { - i--; - kunmap(bio->bi_io_vec[i].bv_page); - } + bio_for_each_segment_all(bvec, bio, i) + kunmap(bvec->bv_page); kfree(mapped_datav); } else if (NULL != dev_state && (bio->bi_opf & REQ_PREFLUSH)) { if (dev_state->state->print_mask & -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: don't poke into bio internals V2 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig ` (7 preceding siblings ...) 2016-11-25 8:07 ` [PATCH 8/8] btrfs: use bio_for_each_segment_all in __btrfsic_submit_bio Christoph Hellwig @ 2016-11-25 9:40 ` David Sterba 8 siblings, 0 replies; 11+ messages in thread From: David Sterba @ 2016-11-25 9:40 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-btrfs, Liu Bo, Omar Sandoval On Fri, Nov 25, 2016 at 09:07:45AM +0100, Christoph Hellwig wrote: > this series has a few patches that switch btrfs to use the proper helpers for > accessing bio internals. This helps to prepare for supporting multi-page > bio_vecs, which are currently under development. > > Changes since v1: > - fixed two compression related bugs > - various minor cleanups > - dropped the last patch of the old series for now - I think > the old code there is already buggy, but it seems like no one > can fully explain to me what it's actually supposed to do.. Added to 4.10 queue. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-11-28 21:38 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-25 8:07 don't poke into bio internals V2 Christoph Hellwig 2016-11-25 8:07 ` [PATCH 1/8] btrfs: use bio iterators for the decompression handlers Christoph Hellwig 2016-11-25 8:07 ` [PATCH 2/8] btrfs: don't access the bio directly in the raid5/6 code Christoph Hellwig 2016-11-25 8:07 ` [PATCH 3/8] btrfs: don't access the bio directly in the direct I/O code Christoph Hellwig 2016-11-25 8:07 ` [PATCH 4/8] btrfs: don't access the bio directly in btrfs_csum_one_bio Christoph Hellwig 2016-11-25 8:07 ` [PATCH 5/8] btrfs: use bi_size Christoph Hellwig 2016-11-25 8:07 ` [PATCH 6/8] btrfs: calculate end of bio offset properly Christoph Hellwig 2016-11-28 21:38 ` Omar Sandoval 2016-11-25 8:07 ` [PATCH 7/8] btrfs: refactor __btrfs_lookup_bio_sums to use bio_for_each_segment_all Christoph Hellwig 2016-11-25 8:07 ` [PATCH 8/8] btrfs: use bio_for_each_segment_all in __btrfsic_submit_bio Christoph Hellwig 2016-11-25 9:40 ` don't poke into bio internals V2 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).