On Mon, May 25, 2009 at 01:04:55AM -0400, Martin K. Petersen wrote: > >>>>> "Alberto" == Alberto Bertogli <albertito@xxxxxxxxxxxxxx> writes: > > Alberto> While at it, I found that bio_integrity_clone() does not clone > Alberto> neither bip_buf nor bip_size, which already copies the bvec, > Alberto> which should have the same data because it's allocated in > Alberto> bio_integrity_prep(). > > Alberto> Is there any reason I'm missing why they shouldn't be copied in > Alberto> bio_integrity_clone(), as illustrated in the following patch? > > Yes. The bip_buf is strictly an internal housekeeping construct. It > contains a pointer to the kernel buffer that contains the protection > information if the protection was added by the block layer itself (via > bio_integrity_prep()). However, that's not always the case. The > protection information may be passed down from a filesystem or > (eventually) a userland application. So the bip_buf is for use by the > top-level of the block layer exclusively. The bip_vec is what you want > to use for accessing the actual protection information. > > Also, the cloned bio may be truncated or split. In that case the > bip_buf isn't going to match the data bvec. So in any case blindly > cloning bip_buf isn't going to work. > > Right now the integrity vector itself is cloned together with the bio > because of the way the block layer works (advancing bvec offset for > partial completion). However, I'm brewing on a patch that gets rid of > that so we can avoid cloning the vector and thus cut down on memory > allocations as the I/O goes through each remapping layer. With my patch > in place the bip_vec becomes immutable and bip_buf will go away. That makes sense, thanks for the explanation. The case I was thinking about was something like a filesystem calling bio_integrity_get_tag() on a cloned bio, since it depends on having a bip_buf available. But if you're going to remove it altogether then it's a moot question. > I took a quick look at your DM patch last week and I didn't see any > reason why it couldn't hook into the block integrity infrastructure. > Take a look at drivers/scsi/sd_dif.c for clues on how to do that. Thanks, I've already implemented it, and will post an updated patch soon, after I clean it up a little. > Let me know if you have questions... Actually, I have two minor questions, if you don't mind: - What would be a decent name to use in struct blk_integrity for a module such as mine? Is LINUX-DMCSUM-V0-CCITT reasonable? - How can I test the tag functions? From a quick grep I found no in-tree users of bio_integrity_get/set_tag(). Thanks a lot, Alberto -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
