Re: [PATCH 0/3] block: adds padding support to blk_rq_map_user_iov

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

On Fri, Apr 11 2008, FUJITA Tomonori wrote:
> On Fri, 11 Apr 2008 12:55:18 +0200
> Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > On Thu, Apr 10 2008, FUJITA Tomonori wrote:
> > > As discussed [*1], blk_rq_map_user_iov path is broken regarding
> > > padding at the moment. In 2.6.24, libata did padding but libata's
> > > padding code was removed and now libata expects the block layer to do
> > > that.
> > > 
> > > blk_rq_map_user does padding but blk_rq_map_user_iov doesn't so
> > > blk_rq_map_user_iov doesn't work in case libata needs padding (so far
> > > nobody has complained, maybe nobody uses blk_rq_map_user_iov
> > > interface).
> > > 
> > > This patchset adds padding support to blk_rq_map_user_iov. I converted
> > > convert bio_copy_user to bio_copy_user_iov, which uses a temporary
> > > kernel buffers.  blk_rq_map_user_iov uses bio_copy_user_iov when a low
> > > level driver needs padding or a buffer in sg_iovec isn't aligned. We
> > > can safely do padding in blk_rq_map_sg.
> > > 
> > > In the long run, I want to integrate several mapping APIs for PC
> > > commands (and new API should be useful for sg/st/osst) but I need more
> > > time to finish that work.
> > > 
> > > This is against the latest Linus tree. Can we merge this after 2.6.25?
> > 
> > Thanks Tomo, this looks good to me know. I'll queue it up.
> 
> My pleasure!
> 
> BTW, what do you think about a patch to change blk_rq_unmap_user to
> take a request instead of a bio?
> 
> http://marc.info/?l=linux-scsi&m=120716475703416&w=2
> 
> I know that you are unwilling to add somthing to struct request (a
> pointer to bio in this case) but symmetrical mapping APIs are more
> handy (the patch also cleans ups the users of the mapping APIs like
> bsg) and less error prone...

I like symmetric APIs as much as the next guy, but it's not really that
ugly in this case I think. As such I'd be willing to take such a patch
only if it cleans up drivers considerably - that is, they have to go
through lengths to hang on to the bio to pass in for unmapping. Your
patch is just borderline I'd say :-)

-- 
Jens Axboe