Re: [PATCH 4 of 7] block: bio data integrity support

[Martin K. Petersen]

>>>>> "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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/