Re: [PATCHSET pata-2.6] ide: rq->buffer, data, special and misc cleanups

[Boaz Harrosh <bharrosh@panasas.com>]

On 03/30/2009 08:20 PM, Tejun Heo wrote:
> Boaz Harrosh wrote:
>>> P.S.: Boaz, the above patchset implements blk_rq_map_kern_sgl() 
>> What is blk_rq_map_kern_sgl() ? I'm not sure I remember ?
>> What is it's API? and what does it do?
>>
>>> and kills the SCSI layer hack.  That was the issue you were trying
>>> to solve, right?
>> If you post URL of git tree I can have a look and start an early review
>> if you want
> 
> Here's the patchset.  It builds but I haven't even booted it yet, so
> expect some breakages.
> 
>   http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=map-untested
>   git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git map-untested
> 
> Thanks.
> 

Hi Tejun.

I hope you have not been working on stabilizing this code yet, I've just seen
it this mornning (Israel time). It has obvious bugs, in the use of scatterlists.

But before you go running to fix them. Don't. I don't like the scatter-list API
at all. We where working hard to remove them from scsi and I don't like them
for block layer either. 

If it is for my personal needs then all I need is a simple:
"I have this bio please attach it to a request". The reason I have a bio
is because it comes from a filesystem and because I will be needing to use
the raid engines that are bio based.

I must have confused you with my blk_rq_map_pages() patches, these where made just
as an example of the ugliness and fragility of not using bios. It was suppose to be
a bad example, not to be accepted. (Hence the absence of my Singed-off-by:)

Scatterlists are bad because they are two times too fat for what they do here, they
need to be allocated, chained and slabbed, and finally they need to be converted to
a bio. Working directly with a bio is the shortest route.

Looking at the patches it seems you are changing them as I write (git-web is freezing)
So I sending this mail now. I was in the process of looking at them from the beginning.

So far it looks like a very deep change, I have some comments, but it will need a proper
review.

I do not understand what was your intention for the blk_rq_map_kern_sgl() how's
the user for it?

BTW: now you absolutely must do something with the name of blk_rq_map_sg() which does not
do any mapping and does not change request at all.

Boaz