* [patch v2] bio-integrity: use hardware sectors instead of block [not found] <20100507082928.GT27064@bicker> @ 2010-05-07 9:54 ` Dan Carpenter 2010-05-17 19:06 ` [patch v2] bio-integrity: use hardware sectors instead of block layer sectors Martin K. Petersen 0 siblings, 1 reply; 3+ messages in thread From: Dan Carpenter @ 2010-05-07 9:54 UTC (permalink / raw) To: Martin K. Petersen Cc: Jens Axboe, Chuck Ebbert, linux-fsdevel, Alexander Viro, kernel-janitors Smatch tagged this code as suspicious because we never use the "nr_sectors" variable. Looking at the code, we did intend to use "nr_sectors" instead of "sectors" when we call bio_integrity_mark_tail(). The difference between "sectors" and "nr_sectors" is that "sectors" is in terms of 512 byte sectors and "nr_sectors" is in terms of hardware sectors. They are only different for 4k sector devices. Also I changed the name because as Jamie Lokier points out, "that code is so asking for the variable to be called 'hw_sectors'." Signed-off-by: Dan Carpenter <error27@gmail.com> diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c index 612a5c3..d8cd1e2 100644 --- a/fs/bio-integrity.c +++ b/fs/bio-integrity.c @@ -667,16 +667,16 @@ void bio_integrity_trim(struct bio *bio, unsigned int offset, { struct bio_integrity_payload *bip = bio->bi_integrity; struct blk_integrity *bi = bdev_get_integrity(bio->bi_bdev); - unsigned int nr_sectors; + unsigned int hw_sectors; BUG_ON(bip = NULL); BUG_ON(bi = NULL); BUG_ON(!bio_flagged(bio, BIO_CLONED)); - nr_sectors = bio_integrity_hw_sectors(bi, sectors); + hw_sectors = bio_integrity_hw_sectors(bi, sectors); bip->bip_sector = bip->bip_sector + offset; bio_integrity_mark_head(bip, offset * bi->tuple_size); - bio_integrity_mark_tail(bip, sectors * bi->tuple_size); + bio_integrity_mark_tail(bip, hw_sectors * bi->tuple_size); } EXPORT_SYMBOL(bio_integrity_trim); ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [patch v2] bio-integrity: use hardware sectors instead of block layer sectors 2010-05-07 9:54 ` [patch v2] bio-integrity: use hardware sectors instead of block Dan Carpenter @ 2010-05-17 19:06 ` Martin K. Petersen 2011-04-05 20:35 ` [patch v2] bio-integrity: use hardware sectors instead of block Jonathan Nieder 0 siblings, 1 reply; 3+ messages in thread From: Martin K. Petersen @ 2010-05-17 19:06 UTC (permalink / raw) To: Dan Carpenter Cc: Martin K. Petersen, Jens Axboe, Chuck Ebbert, linux-fsdevel, Alexander Viro, kernel-janitors >>>>> "Dan" = Dan Carpenter <error27@gmail.com> writes: Dan, This one bitrotted in my mailbox for a while because it conflicted with something else I was working on and then it slipped through the cracks. Sorry about that. Dan> The difference between "sectors" and "nr_sectors" is that "sectors" Dan> is in terms of 512 byte sectors and "nr_sectors" is in terms of Dan> hardware sectors. They are only different for 4k sector devices. Dan> Also I changed the name because as Jamie Lokier points out, "that Dan> code is so asking for the variable to be called 'hw_sectors'." The change is obviously functionally correct. But I object to the notion of hw_sectors. The 1:1 mapping of DIF tuples and logical blocks is even going away in SBC3. So let's not perpetuate that. I have a patch in my queue that gets rid of all the hw_sector references in the integrity code. I'll make sure to include your fix. So thanks for spotting this. I obviously haven't tested PI drives with a 4KB logical block size in combination with device mapper... -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch v2] bio-integrity: use hardware sectors instead of block 2010-05-17 19:06 ` [patch v2] bio-integrity: use hardware sectors instead of block layer sectors Martin K. Petersen @ 2011-04-05 20:35 ` Jonathan Nieder 0 siblings, 0 replies; 3+ messages in thread From: Jonathan Nieder @ 2011-04-05 20:35 UTC (permalink / raw) To: Martin K. Petersen Cc: Dan Carpenter, Jens Axboe, Chuck Ebbert, linux-fsdevel, Alexander Viro, kernel-janitors Hi, On 2010-05-17, Martin K. Petersen wrote: > The change is obviously functionally correct. But I object to the > notion of hw_sectors. The 1:1 mapping of DIF tuples and logical blocks > is even going away in SBC3. So let's not perpetuate that. > > I have a patch in my queue that gets rid of all the hw_sector references > in the integrity code. I'll make sure to include your fix. > > So thanks for spotting this. I obviously haven't tested PI drives with > a 4KB logical block size in combination with device mapper... I'm just curious: did anything come of this? Nowadays gcc 4.6 warns about the same unused nr_sector var smatch warned about, so new readers are coming to the same question of why this function doesn't use hardware sectors. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-04-05 20:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20100507082928.GT27064@bicker>
2010-05-07 9:54 ` [patch v2] bio-integrity: use hardware sectors instead of block Dan Carpenter
2010-05-17 19:06 ` [patch v2] bio-integrity: use hardware sectors instead of block layer sectors Martin K. Petersen
2011-04-05 20:35 ` [patch v2] bio-integrity: use hardware sectors instead of block Jonathan Nieder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).