linux-kernel - Re: [PATCH 4/5] tcm: Unify UNMAP and WRITE_SAME w/ UNMAP=1 subsystem plugin handling

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: <20101014000341.GA15583@lst.de>
Date:	Thu, 14 Oct 2010 02:03:41 +0200
From:	Christoph Hellwig <hch@....de>
To:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
Cc:	Christoph Hellwig <hch@....de>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	Mike Christie <michaelc@...wisc.edu>,
	Hannes Reinecke <hare@...e.de>,
	James Bottomley <James.Bottomley@...e.de>,
	Boaz Harrosh <bharrosh@...asas.com>,
	Jens Axboe <axboe@...nel.dk>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Douglas Gilbert <dgilbert@...erlog.com>,
	Richard Sharpe <realrichardsharpe@...il.com>
Subject: Re: [PATCH 4/5] tcm: Unify UNMAP and WRITE_SAME w/ UNMAP=1 subsystem plugin handling

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.

> >  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, 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.  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.

> 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.

--
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/