From 92fc8708a2d5b20e317a2cec97056c752a8b7bd4 Mon Sep 17 00:00:00 2001 From: Li Dongyang Date: Tue, 10 Aug 2021 23:20:13 +1000 Subject: [PATCH] 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 Reviewed-on: https://review.whamcloud.com/44548 Tested-by: jenkins Reviewed-by: Andreas Dilger Reviewed-by: Li Xi Reviewed-by: Alex Zhuravlev Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/osd-ldiskfs/osd_integrity.c | 76 ++++++++++++++++++++++---------------- lustre/osd-ldiskfs/osd_io.c | 14 +++++-- lustre/tests/sanity.sh | 45 ++++++++++++++++++++++ 3 files changed, 100 insertions(+), 35 deletions(-) diff --git a/lustre/osd-ldiskfs/osd_integrity.c b/lustre/osd-ldiskfs/osd_integrity.c index 3a6433d..56d632c 100644 --- 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 */ }; +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. @@ -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; - 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++) { - if (lnb->lnb_guard_rpc) { + if (lnb && lnb->lnb_guard_rpc) { 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; - 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; @@ -124,14 +138,17 @@ static int osd_dif_type1_verify(struct blk_integrity_exchg *bix, return -EIO; } - *guard_buf = csum; - guard_buf++; + if (guard_buf) { + *guard_buf = csum; + guard_buf++; + } buf += bix->sector_size; sector++; } - lnb->lnb_guard_disk = 1; + if (lnb) + lnb->lnb_guard_disk = 1; 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; - 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++) { - if (lnb->lnb_guard_rpc) { + if (lnb && lnb->lnb_guard_rpc) { 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; - 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; @@ -215,14 +224,17 @@ static int osd_dif_type3_verify(struct blk_integrity_exchg *bix, return -EIO; } - *guard_buf = csum; - guard_buf++; + if (guard_buf) { + *guard_buf = csum; + guard_buf++; + } buf += bix->sector_size; sector++; } - lnb->lnb_guard_disk = 1; + if (lnb) + lnb->lnb_guard_disk = 1; return 0; } diff --git a/lustre/osd-ldiskfs/osd_io.c b/lustre/osd-ldiskfs/osd_io.c index 59a92eb..d72896e 100644 --- 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 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 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) { - 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) { @@ -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++; + lnb = NULL; } return 0; } diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 0158353..b000065 100755 --- 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" +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 -- 1.8.3.1