linux-kernel - Re: [PATCH] [Target_Core_Mod/pSCSI]: Add block/blk-map.c:blk_rq_map_kern

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] [day] [month] [year] [list]

Message-ID: <49FD9CF3.5070005@panasas.com>
Date:	Sun, 03 May 2009 16:32:35 +0300
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
CC:	linux-scsi <linux-scsi@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>, Tejun Heo <tj@...nel.org>,
	James Bottomley <James.Bottomley@...senPartnership.com>,
	Hannes Reinecke <hare@...e.de>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	Mike Christie <michaelc@...wisc.edu>,
	Douglas Gilbert <dgilbert@...erlog.com>,
	Christoph Hellwig <hch@....de>,
	Jens Axboe <jens.axboe@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Vladislav Bolkhovitin <vst@...b.net>
Subject: Re: [PATCH] [Target_Core_Mod/pSCSI]: Add	block/blk-map.c:blk_rq_map_kern_sg()
 usage

On 04/30/2009 10:10 PM, Nicholas A. Bellinger wrote:
> On Thu, 2009-04-30 at 12:37 +0300, Boaz Harrosh wrote:
>>> Currently *without* the blk-map patches on v2.6.30-rc3, is target_core_mod/pSCSI export is
>>> limited to TYPE_DISK and TYPE_ROM that reference a struct block_device using the ConfigFS
>>> 'file descriptor' method.  This is because bio_add_page() expects struct block_device to be
>> Better use bio_add_pc_page(). bio_add_page is only meant for fs requests.
>>
> 
> <nod>, I was only using bio_add_page() in the pre v2.6.30 code for
> target_core_mod/pSCSI because bio_add_pc_page() is not exported from
> fs/bio.c.  Perhaps since bio_add_pc_page() is intended to be for SCSI
> target mode with struct request it should be EXPORT_SYMBOL_GPL()..?
> 

It is very much exported with plain EXPORT_SYMBOL(). If not then I would have
trouble in exofs as a module and so will lots of other Kernel modules. It is
used in more then one place.

>>> each struct bio associated with the struct request w/o Tejun's blk_rq_map_kern_sg().
>>>
>>> Thanks Tejun for this patch series!  Things have been stable so far and I hope to try some
>>> 'bare-metal' and IOV enabled Linux/SCSI target exports using this patch series, along with validating
>>> blk-map on some non TYPE_DISK exports using target_core_mod/pSCSI.  I believe you intend this series for
>>> v2.6.31 correct..? 
>>>
>>> Boaz, have you had a chance to port your stuff over to this yet..?  Other comments..?
>>>
>> No. As I said, these patches were not good for me. I do not have scatterlist at all.
>> I have a pre-made bio, both from filesystem and a block device. So my needs are different.
> 
> Understood..
> 
>> Please note that the patches as last sent, were not good in my opinion, for regressing on
>> some robustness and performance issues.
>>
>> There might be another solution for you, BTW. I'll be reposting a James Bottomley's
>> patch in 1-2 days that makes blk_rq_map_kern() append the buffers it receives instead
>> of only supporting a single call. So you can allocate the request and call blk_rq_map_kern()
>> in a loop for-each-sg. The bad thing with this is that the biovec inside will be allocated
>> multiple times, jumping from small pools to bigger ones.
> 
> Ok, I will plan on testing both methods (single call
> blk_rq_map_kern_sg() vs. appending buffers with blk_rq_map_kern()) using
> the pSCSI export on v2.6.30-rc3..
> 
>>  If only there was a way to specify a pre-allocated bio-size.
> 
> Hrrmm, can you explain a bit more about what this would entail..?  From
> the SCSI target API side, mapping a contigious array of struct

"contiguous" scatterlist array is only up to 128 max on x86_64. This is
the trivial case that is easy to implement with direct bio_xxx exported
calls. The problem start when we need chained scatterlist, to chained
bios, and do not want to invent the block-layer for that.

> scatterlist's from the caller into struct request (and struct bio) in
> place of bvec would still make the most sense I think.
> 

If you want to consider long term. Then I think Tejum's suggestion is best.
That is: To separate scatterlist into a virtual-part and physical-dma-part.
The former just being a reincarnation of bvec (Held at bio as you imagined)
and the later filled and returned by IOMMUs.

> In the OSD case where you are already passing into pre-formatted bio's
> it would be up to the caller to format and validate the your pages via
> an internally allocated (or preallocated) array of scatterlists.
> 

No, in the OSD case I receive a bio that was either built by the block-layer
itself, in the case of a stacking block device, or one built from a filesystem
with the use of exported bio API, that is bio_alloc+bio_add_pc_page*128.

So if bio will use the new-scatterlist internally, it will not affect any of
OSD code since it tries not to touch any bio internal parts.

> Anyways, I will think about it some more and see what can be found..
> 

Yes, time will tell. One good thing is that things are chainging and
a solution will present itself.

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