lkml.org 
[lkml]   [2008]   [Jan]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] bsg : Add support for io vectors in bsg
From
Date
On Tue, 8 Jan 2008 17:09:18 -0500
Pete Wyckoff <pw@osc.edu> wrote:

> tomof@acm.org wrote on Sat, 05 Jan 2008 14:01 +0900:
> > From: Deepak Colluru <deepakrc@gmail.com>
> > Subject: [PATCH] bsg : Add support for io vectors in bsg
> > Date: Fri, 4 Jan 2008 21:47:34 +0530 (IST)
> >
> > > From: Deepak Colluru <deepakrc@gmail.com>
> > >
> > > Add support for io vectors in bsg.
> > >
> > > Signed-off-by: Deepak Colluru <deepakrc@gmail.com>
> > > ---
> > > bsg.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
> > > 1 file changed, 49 insertions(+), 3 deletions(-)
> >
> > Thanks, but I have to NACK this.
> >
> > You can find the discussion about bsg io vector support and a similar
> > patch in linux-scsi archive. I have no plan to support it since it
> > needs the compat hack.
>
> You may recall this is one of the patches I need to use bsg with OSD
> devices. OSDs overload the SCSI buffer model to put mulitple fields
> in dataout and datain. Some is user data, but some is more
> logically created by a library. Memcpying in userspace to wedge all
> the segments into a single buffer is painful, and is required both
> on outgoing and incoming data buffers.
>
> There are two approaches to add iovec to bsg.
>
> 1. Define a new sg_iovec_v4 that uses constant width types. Both
> 32- and 64-bit userspace would hand arrays of this to the kernel.
>
> struct sg_v4_iovec {
> __u64 iov_base;
> __u32 iov_len;
> __u32 __pad1;
> };
>
> Old patch here: http://article.gmane.org/gmane.linux.scsi/30461/

As I said before, I don't think that inventing a new "iovec" is a good
idea. sgv3 use the common "iovec". In addition, sg_io_v4 can be used
by other OSes like sg_io_v3.


> 2. Do as Deepak has done, using the existing sg_iovec, but then
> also work around the compat issue. Old v3 sg_iovec is:
>
> typedef struct sg_iovec /* same structure as used by readv() Linux system */
> { /* call. It defines one scatter-gather element. */
> void __user *iov_base; /* Starting address */
> size_t iov_len; /* Length in bytes */
> } sg_iovec_t;
>
> Old patch here: http://article.gmane.org/gmane.linux.scsi/30460/
>
> I took another look at the compat approach, to see if it is feasible
> to keep the compat handling somewhere else, without the use of #ifdef
> CONFIG_COMPAT and size-comparison code inside bsg.c. I don't see how.
> The use of iovec is within a write operation on a char device. It's
> not amenable to a compat_sys_ or a .compat_ioctl approach.
>
> I'm partial to #1 because the use of architecture-independent fields
> matches the rest of struct sg_io_v4. But if you don't want to have
> another iovec type in the kernel, could we do #2 but just return
> -EINVAL if the need for compat is detected? I.e. change
> dout_iovec_count to dout_iovec_length and do the math?

If you are ok with removing the write/read interface and just have
ioctl, we could can handle comapt stuff like others do. But I think
that you (OSD people) really want to keep the write/read
interface. Sorry, I think that there is no workaround to support iovec
in bsg.


\
 
 \ /
  Last update: 2008-01-09 01:13    [W:0.045 / U:32.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site