All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Christie <michaelc@cs.wisc.edu>
To: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: dougg@torque.net, tomof@acm.org, linux-scsi@vger.kernel.org,
	James.Bottomley@SteelEye.com, jens.axboe@oracle.com
Subject: Re: [PATCH] sg: increase sglist_len of the sg_scatter_hold structure
Date: Wed, 08 Aug 2007 11:58:14 -0500	[thread overview]
Message-ID: <46B9F626.9020200@cs.wisc.edu> (raw)
In-Reply-To: <20070808073801U.fujita.tomonori@lab.ntt.co.jp>

FUJITA Tomonori wrote:
> On Tue, 07 Aug 2007 12:13:41 -0500
> Mike Christie <michaelc@cs.wisc.edu> wrote:
> 
>> FUJITA Tomonori wrote:
>>> Allocating 64K contiguous memory is not good so the next thing to do
>>> is converting sg to use the sg chaining support fully. Or it might be
>> For LLDs like aic7xxx, I think we are stuck with a small 
>> scsi_host_template->sg_tablesize, so to continue to get large requests 
>> like before will we have to still allocate large segments?
> 
> No. sg.c has:
> 
> sizeof(struct scatterlist) * min(q->max_hw_segments, q->max_phys_segments)
> 
> If a lld has small max_hw_segments, it doesn't allocate big contiguous
> memory.
> 

Are we talking about the same thing? Are you saying that it does not 
allocate big continuous memory for the scatterlist right? I was asking 
about continuous memory for the buffer sg.c copies data to/from for the 
IO operation. I was saying that currently for something like aic if we 
want to continue to support 8 MB commands (or whatever it was) like 
before, then because its sg_tablesize/max_hw_segments is so small we 
have to continue allocating large IO buffers. That did not change right? 
If so let me know because you save me :)


> 
>> Is block/scsi_ioctl.c converted to sg chaining in any tree yet? Is that 
>> in your tree or one of Jen's branches.
> 
> block/scsi_ioctl.c uses the standard block layer functions, there is
> nothing to convert in it. sglist doesn't change the standard block
> layer functions much since it doesn't allocate sg list. It changes
> only blk_rq_map_sg.
> 
> Now only scsi-ml is changed to allocate chaining sg list
> properly. Others like cciss are not converted yet, I think. It might
> make sense to have the standard block layer functions to allocate
> chaining sg list properly. So we could convert to potential consumers
> (scsi-ml, sg, ccisss, etc) use them though I'm not sure how many non
> scsi-ml needs chaining sg list.

For drivers like sg and st, do mean the the sg list that is passed to 
functions like scsi_execute_async? If we kill that argument, and instead 
have sg.c and other scsi_execute_async callers just call blk helpers 
like blk_rq_map_user then we would not have to worry about drivers like 
sg needing to know about chaining right? I mean sg.c would not every 
interact with a scatterlist. It would just interact with a request and 
the blk helpers map data for it. The scatterlist that sg and st interact 
with is bogus. It gets thrown away in scsi_execute_async and is only 
used for book keeping.

I think it would be best to either have drivers like sg and st use 
blk_rq helpers to map data to requests like in my patches (this way they 
never know about scatterlists), or have sg and st allocate and setup a 
scatterlist properly and then just attach that to the request like we 
did before (we would have to add back in those checks to scsi-ml to 
check for that case again for this latter idea).

cciss is different right since it is doing blk_rq_map_sg and that 
itself. I am not getting into that :)

  parent reply	other threads:[~2007-08-08 16:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-05 15:31 [PATCH] sg: increase sglist_len of the sg_scatter_hold structure FUJITA Tomonori
2007-08-05 16:55 ` Douglas Gilbert
2007-08-06  4:09   ` FUJITA Tomonori
2007-08-07 17:13     ` Mike Christie
2007-08-07 22:38       ` FUJITA Tomonori
2007-08-08  7:15         ` Jens Axboe
2007-08-08 16:58         ` Mike Christie [this message]
2007-08-08 17:20           ` Mike Christie
2007-08-09  7:33             ` Benny Halevy
2007-08-09 13:43             ` FUJITA Tomonori
2007-08-09 13:43           ` FUJITA Tomonori

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=46B9F626.9020200@cs.wisc.edu \
    --to=michaelc@cs.wisc.edu \
    --cc=James.Bottomley@SteelEye.com \
    --cc=dougg@torque.net \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=jens.axboe@oracle.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=tomof@acm.org \
    /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.