On Thu, 2010-10-14 at 02:03 +0200, Christoph Hellwig wrote: > On Wed, Oct 13, 2010 at 01:56:08PM -0700, Nicholas A. Bellinger wrote: > > > The parsing of the WRITE SAME and UNMAP CDBs is something the generic > > > CDB parsing code should do, > > > > Ok, so you are thinking about a seperate transport_emulate_write_same() > > and transport_emulate_unmap() called from > > transport_emulate_control_cdb(), right..? > > More or less yes. > Ok, then I shall convert transport_generic_[unmap,write_same]() which currently call blk_issue_discard() directly from IBLOCK code, and turn the ->do_discard() subsystem API op into the LBA+Range subsystem call with the underlying IBLOCK specific call to blk_issue_discard(). > > > and just give a range of lists of lba/len > > > pairs to the ->discard method in the backed. > > > > Yes, these are already available from the passed struct > > se_task->task_lba and ->task_size values. > > Not for UNMAP. WRITE SAME in it's various incarnations uses the > standard LBA/LEN encoding and you seem to parse it nicely. But for > UNMAP the lba/len pairs are in the command payload. To support things > genericly you'd need a standard way to pass them. If you want to > limit yourself to one lba/len pair for one the scheme could work, > though. > Yes, this is what transport_generic_unmap() is currently doing when called from iblock_do_discard() an walks the received UNMAP payload. > > Yes, so the problem of trying to make this code generic (eg: outside of > > TCM subsystem plugins) is that blk_issue_discard() takes struct > > block_device, which means we the subsystem plugin has to locate struct > > block_device inside of non generic cide. > > blk_issue_discard is in no way generic. It's 100% iblock code and > really doesn't belong into any other backend. Agreed. > And btw, > blk_issue_discard is rather suboptimal even in iblock - it's a > synchronous function that will stall progress of the thread handling it. > If you want better performance you'll need to opencode the content of > it to allow an asynchronous completion handler. But given that discard > isn't really a critical feature at this point this could easily be > left for later with a comment. > I have not gotten around to the async discard caller just yet, but this is straight-forward enough for the next round.. > > So, then the main issue becomes FILEIO + block level discard and how to > > issue an blk_issue_discard() from struct fileio in the most sane way. > > If there is no sane way then I will just drop this bit, or just do the > > file level 'hole punch' that you are speaking about. > > Right now there is no good way to do a block device discard or file > hole punch at the level where the file backend operates. > Understood. In that case I will go ahead and drop the FILEIO discard support all together for .37 code, and we can revist as necessary down the road. Best, --nab -- 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
