LU-14924 osd-ldiskfs: fix T10PI verify/generate_fn

We are making wrong assumptions in the verify/generate_fn
of T10PI.

Consider this case: we have 4 pages lnb[0-3] in osd_iobuf.
lnb[2] is mapped to a hole, so it won't be added to bio.
If lnb[3] happens to be contiguous after lnb[1], lnb[3] will
be added to bio, with a bi_idx of 2.
In the verify/generate_fn, we work out which niobuf_local
to feed the guard tags to using bi_idx and obp_start_page_idx
and we will end up with wrong niobuf and set the guard tags
for lnb[2].

Contiguous blocks in bio doesn't necessarily mean we are looking
at contiguous niobuf_local/lnb in osd_iobuf->dr_lnbs

Test-Parameters: env=ONLY=77n testlist=sanity
Change-Id: I1ea1b6498692044e680c8754cd31e2c2b7bc9539
Signed-off-by: Li Dongyang <dongyangli@ddn.com>
Reviewed-on: https://review.whamcloud.com/44548
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Li Xi <lixi@ddn.com>
Reviewed-by: Alex Zhuravlev <bzzz@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/osd-ldiskfs/osd_integrity.c b/lustre/osd-ldiskfs/osd_integrity.c
index 3a6433d..56d632c 100644 (file)
--- a/lustre/osd-ldiskfs/osd_integrity.c
+++ b/lustre/osd-ldiskfs/osd_integrity.c
@@ -45,6 +45,28 @@ struct sd_dif_tuple {
        __be32 ref_tag;          /* Target LBA or indirect LBA */
 };
 
        __be32 ref_tag;          /* Target LBA or indirect LBA */
 };
 

+static struct niobuf_local *find_lnb(struct blk_integrity_exchg *bix)
+{
+       struct bio *bio = bix->bio;
+       struct bio_vec *bv = bio_iovec_idx(bio, bix->bi_idx);
+       struct osd_bio_private *bio_private = bio->bi_private;
+       struct osd_iobuf *iobuf = bio_private->obp_iobuf;
+       int index = bio_private->obp_start_page_idx + bix->bi_idx;
+       int i;
+
+       /*
+        * blocks are contiguous in bio but pages added to bio
+        * could have a gap comparing to iobuf->dr_pages.
+        * e.g. a page mapped to a hole in the middle.
+        */
+       for (i = index; i < iobuf->dr_npages; i++) {
+               if (iobuf->dr_pages[i] == bv->bv_page)
+                       return iobuf->dr_lnbs[i];
+       }
+
+       return NULL;
+}
+

 /*
  * Type 1 and Type 2 protection use the same format: 16 bit guard tag,
  * 16 bit app tag, 32 bit reference tag.
 /*
  * Type 1 and Type 2 protection use the same format: 16 bit guard tag,
  * 16 bit app tag, 32 bit reference tag.


@@ -54,17 +76,13 @@ static void osd_dif_type1_generate(struct blk_integrity_exchg *bix,
 {
        void *buf = bix->data_buf;
        struct sd_dif_tuple *sdt = bix->prot_buf;
 {
        void *buf = bix->data_buf;
        struct sd_dif_tuple *sdt = bix->prot_buf;

-       struct bio *bio = bix->bio;
-       struct osd_bio_private *bio_private = bio->bi_private;
-       struct osd_iobuf *iobuf = bio_private->obp_iobuf;
-       int index = bio_private->obp_start_page_idx + bix->bi_idx;
-       struct niobuf_local *lnb = iobuf->dr_lnbs[index];
-       __u16 *guard_buf = lnb->lnb_guards;
+       struct niobuf_local *lnb = find_lnb(bix);
+       __u16 *guard_buf = lnb ? lnb->lnb_guards : NULL;

        sector_t sector = bix->sector;
        unsigned int i;
 
        for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
        sector_t sector = bix->sector;
        unsigned int i;
 
        for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {

-               if (lnb->lnb_guard_rpc) {
+               if (lnb && lnb->lnb_guard_rpc) {

                        sdt->guard_tag = *guard_buf;
                        guard_buf++;
                } else
                        sdt->guard_tag = *guard_buf;
                        guard_buf++;
                } else


@@ -92,12 +110,8 @@ static int osd_dif_type1_verify(struct blk_integrity_exchg *bix,
 {
        void *buf = bix->data_buf;
        struct sd_dif_tuple *sdt = bix->prot_buf;
 {
        void *buf = bix->data_buf;
        struct sd_dif_tuple *sdt = bix->prot_buf;

-       struct bio *bio = bix->bio;
-       struct osd_bio_private *bio_private = bio->bi_private;
-       struct osd_iobuf *iobuf = bio_private->obp_iobuf;
-       int index = bio_private->obp_start_page_idx + bix->bi_idx;
-       struct niobuf_local *lnb = iobuf->dr_lnbs[index];
-       __u16 *guard_buf = lnb->lnb_guards;
+       struct niobuf_local *lnb = find_lnb(bix);
+       __u16 *guard_buf = lnb ? lnb->lnb_guards : NULL;

        sector_t sector = bix->sector;
        unsigned int i;
        __u16 csum;
        sector_t sector = bix->sector;
        unsigned int i;
        __u16 csum;


@@ -124,14 +138,17 @@ static int osd_dif_type1_verify(struct blk_integrity_exchg *bix,
                        return -EIO;
                }
 
                        return -EIO;
                }
 

-               *guard_buf = csum;
-               guard_buf++;
+               if (guard_buf) {
+                       *guard_buf = csum;
+                       guard_buf++;
+               }

 
                buf += bix->sector_size;
                sector++;
        }
 
 
                buf += bix->sector_size;
                sector++;
        }
 

-       lnb->lnb_guard_disk = 1;
+       if (lnb)
+               lnb->lnb_guard_disk = 1;

        return 0;
 }
 
        return 0;
 }
 


@@ -154,16 +171,12 @@ static void osd_dif_type3_generate(struct blk_integrity_exchg *bix,
 {
        void *buf = bix->data_buf;
        struct sd_dif_tuple *sdt = bix->prot_buf;
 {
        void *buf = bix->data_buf;
        struct sd_dif_tuple *sdt = bix->prot_buf;

-       struct bio *bio = bix->bio;
-       struct osd_bio_private *bio_private = bio->bi_private;
-       struct osd_iobuf *iobuf = bio_private->obp_iobuf;
-       int index = bio_private->obp_start_page_idx + bix->bi_idx;
-       struct niobuf_local *lnb = iobuf->dr_lnbs[index];
-       __u16 *guard_buf = lnb->lnb_guards;
+       struct niobuf_local *lnb = find_lnb(bix);
+       __u16 *guard_buf = lnb ? lnb->lnb_guards : NULL;

        unsigned int i;
 
        for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {
        unsigned int i;
 
        for (i = 0 ; i < bix->data_size ; i += bix->sector_size, sdt++) {

-               if (lnb->lnb_guard_rpc) {
+               if (lnb && lnb->lnb_guard_rpc) {

                        sdt->guard_tag = *guard_buf;
                        guard_buf++;
                } else
                        sdt->guard_tag = *guard_buf;
                        guard_buf++;
                } else


@@ -190,12 +203,8 @@ static int osd_dif_type3_verify(struct blk_integrity_exchg *bix,
 {
        void *buf = bix->data_buf;
        struct sd_dif_tuple *sdt = bix->prot_buf;
 {
        void *buf = bix->data_buf;
        struct sd_dif_tuple *sdt = bix->prot_buf;

-       struct bio *bio = bix->bio;
-       struct osd_bio_private *bio_private = bio->bi_private;
-       struct osd_iobuf *iobuf = bio_private->obp_iobuf;
-       int index = bio_private->obp_start_page_idx + bix->bi_idx;
-       struct niobuf_local *lnb = iobuf->dr_lnbs[index];
-       __u16 *guard_buf = lnb->lnb_guards;
+       struct niobuf_local *lnb = find_lnb(bix);
+       __u16 *guard_buf = lnb ? lnb->lnb_guards : NULL;

        sector_t sector = bix->sector;
        unsigned int i;
        __u16 csum;
        sector_t sector = bix->sector;
        unsigned int i;
        __u16 csum;


@@ -215,14 +224,17 @@ static int osd_dif_type3_verify(struct blk_integrity_exchg *bix,
                        return -EIO;
                }
 
                        return -EIO;
                }
 

-               *guard_buf = csum;
-               guard_buf++;
+               if (guard_buf) {
+                       *guard_buf = csum;
+                       guard_buf++;
+               }

 
                buf += bix->sector_size;
                sector++;
        }
 
 
                buf += bix->sector_size;
                sector++;
        }
 

-       lnb->lnb_guard_disk = 1;
+       if (lnb)
+               lnb->lnb_guard_disk = 1;

        return 0;
 }
 
        return 0;
 }
 

diff --git a/lustre/osd-ldiskfs/osd_io.c b/lustre/osd-ldiskfs/osd_io.c
index 59a92eb..d72896e 100644 (file)
--- a/lustre/osd-ldiskfs/osd_io.c
+++ b/lustre/osd-ldiskfs/osd_io.c
@@ -320,20 +320,27 @@ static int osd_bio_integrity_compare(struct bio *bio, struct block_device *bdev,
 {
        struct blk_integrity *bi = bdev_get_integrity(bdev);
        struct bio_integrity_payload *bip = bio->bi_integrity;
 {
        struct blk_integrity *bi = bdev_get_integrity(bdev);
        struct bio_integrity_payload *bip = bio->bi_integrity;

-       struct niobuf_local *lnb;
+       struct niobuf_local *lnb = NULL;

        unsigned short sector_size = blk_integrity_interval(bi);
        void *bio_prot_buf = page_address(bip->bip_vec->bv_page) +
                bip->bip_vec->bv_offset;
        struct bio_vec *bv;
        sector_t sector = bio_start_sector(bio);
        unsigned short sector_size = blk_integrity_interval(bi);
        void *bio_prot_buf = page_address(bip->bip_vec->bv_page) +
                bip->bip_vec->bv_offset;
        struct bio_vec *bv;
        sector_t sector = bio_start_sector(bio);

-       unsigned int sectors, total;
+       unsigned int i, sectors, total;

        DECLARE_BVEC_ITER_ALL(iter_all);
        __u16 *expected_guard;
        int rc;
 
        total = 0;
        bio_for_each_segment_all(bv, bio, iter_all) {
        DECLARE_BVEC_ITER_ALL(iter_all);
        __u16 *expected_guard;
        int rc;
 
        total = 0;
        bio_for_each_segment_all(bv, bio, iter_all) {

-               lnb = iobuf->dr_lnbs[index];
+               for (i = index; i < iobuf->dr_npages; i++) {
+                       if (iobuf->dr_pages[i] == bv->bv_page) {
+                               lnb = iobuf->dr_lnbs[i];
+                               break;
+                       }
+               }
+               if (!lnb)
+                       continue;

                expected_guard = lnb->lnb_guards;
                sectors = bv->bv_len / sector_size;
                if (lnb->lnb_guard_rpc) {
                expected_guard = lnb->lnb_guards;
                sectors = bv->bv_len / sector_size;
                if (lnb->lnb_guard_rpc) {


@@ -348,6 +355,7 @@ static int osd_bio_integrity_compare(struct bio *bio, struct block_device *bdev,
                total += sectors * bi->tuple_size;
                LASSERT(total <= bip_size(bio->bi_integrity));
                index++;
                total += sectors * bi->tuple_size;
                LASSERT(total <= bip_size(bio->bi_integrity));
                index++;

+               lnb = NULL;

        }
        return 0;
 }
        }
        return 0;
 }

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 0158353..b000065 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -9653,6 +9653,51 @@ test_77m() {
 }
 run_test 77m "Verify checksum_speed is correctly read"
 
 }
 run_test 77m "Verify checksum_speed is correctly read"
 

+check_filefrag_77n() {
+       local nr_ext=0
+       local starts=()
+       local ends=()
+
+       while read extidx a b start end rest; do
+               if [[ "${extidx}" =~ ^[0-9]+: ]]; then
+                       nr_ext=$(( $nr_ext + 1 ))
+                       starts+=( ${start%..} )
+                       ends+=( ${end%:} )
+               fi
+       done < <( filefrag -sv $1 )
+
+       [[ $nr_ext -eq 2 ]] && [[ "${starts[-1]}" == $(( ${ends[0]} + 1 )) ]] && return 0
+       return 1
+}
+
+test_77n() {
+       [[ "$CKSUM_TYPES" =~ t10 ]] || skip "no T10 checksum support on osc"
+
+       touch $DIR/$tfile
+       $TRUNCATE $DIR/$tfile 0
+       dd if=/dev/urandom of=$DIR/$tfile bs=4k conv=notrunc count=1 seek=0
+       dd if=/dev/urandom of=$DIR/$tfile bs=4k conv=notrunc count=1 seek=2
+       check_filefrag_77n $DIR/$tfile ||
+               skip "$tfile blocks not contiguous around hole"
+
+       set_checksums 1
+       stack_trap "set_checksums $ORIG_CSUM" EXIT
+       stack_trap "set_checksum_type $ORIG_CSUM_TYPE" EXIT
+       stack_trap "rm -f $DIR/$tfile"
+
+       for algo in $CKSUM_TYPES; do
+               if [[ "$algo" =~ ^t10 ]]; then
+                       set_checksum_type $algo ||
+                               error "fail to set checksum type $algo"
+                       dd if=$DIR/$tfile of=/dev/null bs=12k count=1 iflag=direct ||
+                               error "fail to read $tfile with $algo"
+               fi
+       done
+       rm -f $DIR/$tfile
+       return 0
+}
+run_test 77n "Verify read from a hole inside contiguous blocks with T10PI"
+

 cleanup_test_78() {
        trap 0
        rm -f $DIR/$tfile
 cleanup_test_78() {
        trap 0
        rm -f $DIR/$tfile