On Wed, 02 Jun 2010 11:54:25 +0300 Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote: > > Some LLDs already use scsi_get_lba(). They don't know scsi command > > from pc or fs so they can't use scsi_get_lba() now. Well, they can see > > scsi_cmnd->request to know pc or fs but it's layer violation. They > > shouldn't access to the block layer stuff. > > > > right, so hide it all under an API that is implemented at the right > level, and protect them. The driver should call a single simple > API the magic will be done at the generic level > > if (blk_fs_rq(req)) > return blk_rq_pos(req); > else > return scsi_somthing(); The first version that I posted yesterday is something like that. But I don't think that it's clean. > >> Only drivers/code that actually cares should call this member. > >> > >> It should be easy enough to search for __sector and change them > >> to a function call. > > > > I don't think that calling such function in LLDs is a clean design. We > > set request->__sector for fs requests. Why not for pc requests? Then > > we can use blk_rq_pos() cleanly. > > > > Because "fs requests" have a defined blk_rq_pos(). But "pc requests" > don't necessarily have one. Pretending to invent one is stupid and > dangerous. Better keep a -1 for all "pc requests". I don't see your logic, why using blk_rq_pos() for pc is dangerous? > scsi LLDs can be pass-through for disks, as well as lots of other > type of devices. They should not assume the role of a disk. The use > of blk_rq_pos() at LLDs is probably wrong in the first place. Unless > it is some kind of scsi emulation driver like scsi_debug. Not only scsi_debug. Have you seen how scsi_get_lba() is used? LLDs sometime needs to know lba in scsi commands. How about calling this function in sd_prep_fn, sr_prep_fn instead of scsi_setup_blk_pc_cmnd? Then it doesn't affect OSD. -- 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
