From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e23smtp09.au.ibm.com ([202.81.31.142]:35136 "EHLO e23smtp09.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932919AbbHILsK (ORCPT ); Sun, 9 Aug 2015 07:48:10 -0400 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sun, 9 Aug 2015 21:48:08 +1000 Received: from d23relay09.au.ibm.com (d23relay09.au.ibm.com [9.185.63.181]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 3EB523578058 for ; Sun, 9 Aug 2015 21:48:05 +1000 (EST) Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay09.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t79BlvNo46202956 for ; Sun, 9 Aug 2015 21:48:05 +1000 Received: from d23av02.au.ibm.com (localhost [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t79BlVMc004069 for ; Sun, 9 Aug 2015 21:47:32 +1000 From: Chandan Rajendra To: Josef Bacik Cc: linux-btrfs@vger.kernel.org, clm@fb.com, bo.li.liu@oracle.com, dsterba@suse.cz, chandan@mykolab.com, quwenruo@cn.fujitsu.com Subject: Re: [PATCH V2 02/11] Btrfs: Compute and look up csums based on sectorsized blocks Date: Sun, 09 Aug 2015 17:17:13 +0530 Message-ID: <1909720.tCRSQLki4e@localhost.localdomain> In-Reply-To: <55C4F93D.3010605@fb.com> References: <1438931158-20888-1-git-send-email-chandan@linux.vnet.ibm.com> <1438931158-20888-3-git-send-email-chandan@linux.vnet.ibm.com> <55C4F93D.3010605@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Sender: linux-btrfs-owner@vger.kernel.org List-ID: On Friday 07 Aug 2015 14:30:21 Josef Bacik wrote: > On 08/07/2015 03:05 AM, Chandan Rajendra wrote: > > Checksums are applicable to sectorsize units. The current code uses > > bio->bv_len units to compute and look up checksums. This works on machines > > where sectorsize == PAGE_SIZE. This patch makes the checksum computation > > and look up code to work with sectorsize units. > > > > Reviewed-by: Liu Bo > > Signed-off-by: Chandan Rajendra > > --- > > > > fs/btrfs/file-item.c | 90 > > +++++++++++++++++++++++++++++++++------------------- 1 file changed, 57 > > insertions(+), 33 deletions(-) > > > > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c > > index 58ece65..d752051 100644 > > --- a/fs/btrfs/file-item.c > > +++ b/fs/btrfs/file-item.c > > @@ -172,6 +172,7 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root > > *root,> > > u64 item_start_offset = 0; > > u64 item_last_offset = 0; > > u64 disk_bytenr; > > > > + u64 page_bytes_left; > > > > u32 diff; > > int nblocks; > > int bio_index = 0; > > > > @@ -220,6 +221,8 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root > > *root,> > > disk_bytenr = (u64)bio->bi_iter.bi_sector << 9; > > if (dio) > > > > offset = logical_offset; > > > > + > > + page_bytes_left = bvec->bv_len; > > > > while (bio_index < bio->bi_vcnt) { > > > > if (!dio) > > > > offset = page_offset(bvec->bv_page) + bvec->bv_offset; > > > > @@ -243,7 +246,7 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root > > *root,> > > if (BTRFS_I(inode)->root->root_key.objectid == > > > > BTRFS_DATA_RELOC_TREE_OBJECTID) { > > > > set_extent_bits(io_tree, offset, > > > > - offset + bvec->bv_len - 1, > > + offset + root->sectorsize - 1, > > > > EXTENT_NODATASUM, GFP_NOFS); > > > > } else { > > > > btrfs_info(BTRFS_I(inode)->root- >fs_info, > > > > @@ -281,11 +284,17 @@ static int __btrfs_lookup_bio_sums(struct btrfs_root > > *root,> > > found: > > csum += count * csum_size; > > nblocks -= count; > > > > - bio_index += count; > > + > > > > while (count--) { > > > > - disk_bytenr += bvec->bv_len; > > - offset += bvec->bv_len; > > - bvec++; > > + disk_bytenr += root->sectorsize; > > + offset += root->sectorsize; > > + page_bytes_left -= root->sectorsize; > > + if (!page_bytes_left) { > > + bio_index++; > > + bvec++; > > + page_bytes_left = bvec->bv_len; > > + } > > + > > > > } > > > > } > > btrfs_free_path(path); > > > > @@ -432,6 +441,8 @@ int btrfs_csum_one_bio(struct btrfs_root *root, struct > > inode *inode,> > > struct bio_vec *bvec = bio->bi_io_vec; > > int bio_index = 0; > > int index; > > > > + int nr_sectors; > > + int i; > > > > unsigned long total_bytes = 0; > > unsigned long this_sum_bytes = 0; > > u64 offset; > > > > @@ -459,41 +470,54 @@ int btrfs_csum_one_bio(struct btrfs_root *root, > > struct inode *inode,> > > if (!contig) > > > > offset = page_offset(bvec->bv_page) + bvec->bv_offset; > > > > - if (offset >= ordered->file_offset + ordered->len || > > - offset < ordered->file_offset) { > > - unsigned long bytes_left; > > - sums->len = this_sum_bytes; > > - this_sum_bytes = 0; > > - btrfs_add_ordered_sum(inode, ordered, sums); > > - btrfs_put_ordered_extent(ordered); > > + data = kmap_atomic(bvec->bv_page); > > I don't think we can have something kmap_atomic()'ed and then do > allocations under it right? That's why we only kmap_atomic(), do the > copy, and then unmap, unless I'm forgetting something? Thanks, > Josef, you are correct. I will fix it and send out version V3 of the patchset soon. Thanks for the review. -- chandan