From: Boaz Harrosh <bharrosh@panasas.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Jens Axboe <axboe@kernel.dk>,
linux-raid@vger.kernel.org, dm-devel@redhat.com,
linux-kernel@vger.kernel.org, nab@linux-iscsi.org,
ryanh@us.ibm.com
Subject: Re: [PATCH 1/2] block: export __make_request
Date: Wed, 14 Sep 2011 20:17:53 +0300 [thread overview]
Message-ID: <4E70E1C1.8040907@panasas.com> (raw)
In-Reply-To: <20110913211911.GA13894@infradead.org>
On 9/14/2011 12:19 AM, Christoph Hellwig wrote:
> On Mon, Sep 12, 2011 at 08:04:46PM +0200, Jens Axboe wrote:
>>> I really hate naming things different from the method they are
>>> implementing. I've tried to figure out what the point of the
>>> old blk_make_request is - why would we not go through
>>> generic_make_request for this?
>>>
>>> Boaz, any idea?
>>
>> I tend to agree, we could rename the existing blk_make_request(). It
>> could be blk_make_request_from_bio() or something like that, since
>> that's what it does.
>
> It should at very least be renamed. But I still can't figure out what
> it is for exactly.
>
> There are three users:
>
> (1) virtio_blk::virtblk_get_id():
> This looks like it really should just use blk_rq_map_kern.
> (2) osd_initiator::_make_request():
> This one looks like it should just use the same scheme as
> sg_io(), as it's doing the same thing.
Good god what sg_io? That broken pointr+length from user-mode that sg.c
and bsg.c are using? no can do it's not user-mode pointers, and it's not
pointer+length it's pages pointers of a bio. The only other structure
that could carry the same information is struct sg, but we work very
hard to get rid of this contraption. (scsi_execute_async or something
that it was)
blk_make_request() was made to be the parallel of __make_request, to be
used from filesystem level users. But with two differences.
1. Mainly support for none-FS BLOCK_PC requests
2. Also support chained bios. (was added later)
> (3) target_core_pscsi::__pscsi_map_SG():
> Same as (2).
>
There is no better suitable structure in current Kernel to carry a list
of pages, with optional offset and length, then bio struct. Given a bio
at hand. how do you make a block request out of it? (If it's not an
FS_PC type IO?)
As I remember target_core had their own pages-linked-list structure, and
how do you make a request out of that? again best at hand is bio.
bio is for a long time a page-pointers-carrier-structure and is out of
private block-level use. The filesystem level is full of it.
Thanks
Boaz
WARNING: multiple messages have this Message-ID (diff)
From: Boaz Harrosh <bharrosh@panasas.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Jens Axboe <axboe@kernel.dk>, <linux-raid@vger.kernel.org>,
<dm-devel@redhat.com>, <linux-kernel@vger.kernel.org>,
<nab@linux-iscsi.org>, <ryanh@us.ibm.com>
Subject: Re: [PATCH 1/2] block: export __make_request
Date: Wed, 14 Sep 2011 20:17:53 +0300 [thread overview]
Message-ID: <4E70E1C1.8040907@panasas.com> (raw)
In-Reply-To: <20110913211911.GA13894@infradead.org>
On 9/14/2011 12:19 AM, Christoph Hellwig wrote:
> On Mon, Sep 12, 2011 at 08:04:46PM +0200, Jens Axboe wrote:
>>> I really hate naming things different from the method they are
>>> implementing. I've tried to figure out what the point of the
>>> old blk_make_request is - why would we not go through
>>> generic_make_request for this?
>>>
>>> Boaz, any idea?
>>
>> I tend to agree, we could rename the existing blk_make_request(). It
>> could be blk_make_request_from_bio() or something like that, since
>> that's what it does.
>
> It should at very least be renamed. But I still can't figure out what
> it is for exactly.
>
> There are three users:
>
> (1) virtio_blk::virtblk_get_id():
> This looks like it really should just use blk_rq_map_kern.
> (2) osd_initiator::_make_request():
> This one looks like it should just use the same scheme as
> sg_io(), as it's doing the same thing.
Good god what sg_io? That broken pointr+length from user-mode that sg.c
and bsg.c are using? no can do it's not user-mode pointers, and it's not
pointer+length it's pages pointers of a bio. The only other structure
that could carry the same information is struct sg, but we work very
hard to get rid of this contraption. (scsi_execute_async or something
that it was)
blk_make_request() was made to be the parallel of __make_request, to be
used from filesystem level users. But with two differences.
1. Mainly support for none-FS BLOCK_PC requests
2. Also support chained bios. (was added later)
> (3) target_core_pscsi::__pscsi_map_SG():
> Same as (2).
>
There is no better suitable structure in current Kernel to carry a list
of pages, with optional offset and length, then bio struct. Given a bio
at hand. how do you make a block request out of it? (If it's not an
FS_PC type IO?)
As I remember target_core had their own pages-linked-list structure, and
how do you make a request out of that? again best at hand is bio.
bio is for a long time a page-pointers-carrier-structure and is out of
private block-level use. The filesystem level is full of it.
Thanks
Boaz
next prev parent reply other threads:[~2011-09-14 17:17 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-11 14:50 [PATCH 1/2] block: export __make_request Christoph Hellwig
2011-09-11 14:51 ` [PATCH 2/2] block: remove support for bio remapping from ->make_request Christoph Hellwig
2011-09-12 3:25 ` NeilBrown
2011-09-12 9:59 ` Jens Axboe
2011-09-12 12:25 ` Christoph Hellwig
2011-09-12 12:26 ` Jens Axboe
2011-09-12 9:59 ` [PATCH 1/2] block: export __make_request Jens Axboe
2011-09-12 12:25 ` Christoph Hellwig
2011-09-12 12:26 ` Jens Axboe
2011-09-12 13:38 ` Christoph Hellwig
2011-09-12 18:04 ` Jens Axboe
2011-09-13 21:19 ` Christoph Hellwig
2011-09-14 17:17 ` Boaz Harrosh [this message]
2011-09-14 17:17 ` Boaz Harrosh
2011-09-14 17:53 ` Doug Dumitru
2011-09-14 17:53 ` Doug Dumitru
2011-09-14 18:40 ` Alan Cox
2011-09-14 21:34 ` Doug Dumitru
2011-09-14 22:01 ` Alan Cox
2011-09-14 22:48 ` Doug Dumitru
2011-09-15 10:20 ` Bernd Petrovitsch
2011-09-14 20:16 ` Nicholas A. Bellinger
2011-09-12 13:42 ` [PATCH 3/2] block: refactor generic_make_request Christoph Hellwig
2011-09-12 15:09 ` Vivek Goyal
2011-09-12 15:13 ` Christoph Hellwig
2011-09-15 11:55 ` Christoph Hellwig
2011-09-15 11:56 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E70E1C1.8040907@panasas.com \
--to=bharrosh@panasas.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@redhat.com \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-raid@vger.kernel.org \
--cc=nab@linux-iscsi.org \
--cc=ryanh@us.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.