>>>>> "Jeff" == Jeff Moyer <jmoyer@xxxxxxxxxx> writes: >> + memset(bip, 0, sizeof(*bip)); >> + idx = 0; Jeff> That assignment isn't necessary. Zap! Jeff> nr_sectors = (len + bi->tag_size - 1) / bi->tag_size; Jeff> why not simply use DIV_ROUND_UP? Fixed. Jeff> set_tag and get_tag are almost identical. Any chance you want Jeff> to factor out that code? Done. Jeff> Hmm, up until this point you use bi to mean bio_integrity, but Jeff> now it means blk_integrity. Confusion will ensue. ;) Err, uhm. There is no bio_integrity. There's the bio integrity payload which I always refer to as struct bip *bip. And struct blk_integrity which is always bi. I'm also anal about using bv for the data bio_vec and iv for the integrity bio_vec. I can't see any place where I'm inconsistent. Jeff> struct blk_integrity_exchg is not yet defined in your patch set, Jeff> so this will likely break git bisect. bio-integrity.patch and blk-integrity.patch are artificially split up to ease the review process. They are not meant to be separate changesets. >> + buf = kzalloc(len, GFP_NOIO | q->bounce_gfp); Jeff> Does this actually need to be zeroed? Nope. >> +void bio_integrity_advance(struct bio *bio, unsigned int > +void bio_integrity_trim(struct bio *bio, unsigned int offset, unsigned int sectors) Jeff> The above two loops look pretty much the same to me. Can you Jeff> factor that out to a helper? I've created helpers for marking head and tail of the ivec. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
