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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <yq1r6b4mx9y.fsf@sermon.lab.mkp.net>
Date:	Wed, 11 Jun 2008 00:05:29 -0400
From:	"Martin K. Petersen" <martin.petersen@...cle.com>
To:	Jeff Moyer <jmoyer@...hat.com>
Cc:	"Martin K. Petersen" <martin.petersen@...cle.com>,
	linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org
Subject: Re: [PATCH 4 of 7] block: bio data integrity support

>>>>> "Jeff" == Jeff Moyer <jmoyer@...hat.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/