Re: [PATCH v2] add bidi support for block pc requests

[Jens Axboe <jens.axboe@oracle.com>]

On Thu, May 17 2007, FUJITA Tomonori wrote:
> From: Jens Axboe <jens.axboe@oracle.com>
> Subject: Re: [PATCH v2] add bidi support for block pc requests
> Date: Wed, 16 May 2007 19:53:22 +0200
> 
> > On Wed, May 16 2007, Boaz Harrosh wrote:
> > > Boaz Harrosh wrote:
> > > > James Bottomley wrote:
> > > >> 
> > > >> There's actually a fourth option you haven't considered:
> > > >>
> > > >> Roll all the required sglist definitions (request_bufflen,
> > > >> request_buffer, use_sg and sglist_len) into the sgtable pools.
> > > >>
> > > > This is a grate Idea. Let me see if I understand what you mean.
> > > > ...
> > > > ...
> > > 
> > > Hi Dear James, list.
> > > I have worked on proposed solution (If anyone is interested see
> > > url blow)
> > > 
> > > Now it works and all but I hit some problems.
> > > What happens is that in 64 bit architectures, well in x86_64,
> > > the sizeof scatterlist structure is 32 bytes which means that
> > > we can only fit exactly 128 of them in a page. But together with
> > > the scsi_sg_table header we are down to 127. Also if we want
> > > good alignment/packing of other sized pools we want:
> > > static struct scsi_host_sg_pool scsi_sg_pools[] = {
> > > 	SP(7),
> > > 	SP(15),
> > > 	SP(31),
> > > 	SP(63),
> > > 	SP(127)
> > > };
> > > 
> > > now there are 2 issues with this:
> > > 1. Even if I do blk_queue_max_phys_segments(q, 127); I still get
> > >    requests for use_sg=128 which will crash the kernel.
> > 
> > That sounds like a serious issue, it should definitely not happen. Stuff
> > like that would bite other drivers as well, are you absolutely sure that
> > is happening? Power-of-2 bug in your code, or in the SCSI code?
> 
> Boaz, how do you send requests to the scsi-ml, via fs, sg, or bsg?
> 
> 
> > > If anyone wants to see I have done 2 versions of this work. One on top
> > > of Jens's sg-chaining work (ver 5). And one on top of Tomo's cleanup
> > > git. both can be found here:
> > >   http://www.bhalevy.com/open-osd/download/scsi_sg_table/
> > 
> > +#define scsi_for_each_sg(cmd, sg, nseg, __i)                   \
> > +       for (__i = 0, sg = scsi_sglist(cmd); __i < (nseg); __i++, (sg)++)
> > 
> > Hmm?
> 
> When for_each_sg is ready, we convert scsi_for_each_sg to use
> for_each_sg. I finished the cleanups on more than 40 drivers on the
> top of your patches.

But this is from the patch that is based on my sglist branch, so it
looked somewhat odd.

-- 
Jens Axboe