fujita.tomonori@xxxxxxxxxxxxx wrote on Mon, 19 Mar 2007 23:21 +0900:
> From: Douglas Gilbert <dougg@xxxxxxxxxx>
> Subject: Re: [PATCH] bsg: iovec support
> Date: Mon, 19 Mar 2007 10:04:45 -0400
>
> > >> Pete is also suggesting (shown above) a revised sg_io_vec
> > >> structure that uses a uint64_t for the pointer to simplify
> > >> 32, 64 bit thunking.
> > >
> > > All I said is that it would be better to use the existing compat
> > > infrastructure (sg_build_iovec, sg_ioctl_trans, etc in
> > > fs/compat_ioctl.c) instead of adding another compat code.
> >
> > Won't sg v4 make this even a bigger mess, at least
> > initially anyway?
>
> Inventing a new iovec structure like sg_io_v4_vec and something like
> blk_rq_map_user_iov_sgv4 sounds a mess.
It is sort of a mess to have new blk mapping routine for a new iovec
type. But I very much did not want to introduce the need for
another compat conversion. But, I took a look at how it would be to
pass the existing sg_iovec into bsg.
Adding a new bsg_write_compat would be bad. There is lots of
parsing and setup before we even get to the possibility of iovec
data mapping. Reusing just sg_build_ioctl from compat_ioctl.c is
also suboptimal as this function is built around the idea of a
contiguous sg_io_hdr + iovec in userspace. The function is small
enough that splitting it into a generic + ioctl-specific part would
add too much abstraction to be worth it.
Here is the patch to use sg_iovec, with its userspace void * and
size_t, and the CONFIG_COMPAT code to fixup 32-bit userspace. I'm
not fond of having __u64 for non-iovec buffer representations, and
void * for iovec buffer representations, but it saves having to
build an sg_iovec just to call into the existing blk_rq_map_user_iov.
Comments?
-- Pete
Support vectored IO as in SGv3. This uses the standard struct sg_iovec
(same as struct iovec) and converts if necessary for CONFIG_COMPAT.
Signed-off-by: Pete Wyckoff <pw@xxxxxxx>
---
block/bsg.c | 105 +++++++++++++++++++++++++++++++++++++++-----------
include/linux/bsg.h | 2 +
2 files changed, 84 insertions(+), 23 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index f1ea258..5a31062 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -25,6 +25,7 @@
#include <linux/percpu.h>
#include <linux/uio.h>
#include <linux/bsg.h>
+#include <linux/compat.h>
#include <scsi/scsi.h>
#include <scsi/scsi_ioctl.h>
@@ -280,6 +281,65 @@ bsg_validate_sgv4_hdr(request_queue_t *q, struct sg_io_v4 *hdr, int *rw)
}
/*
+ * Map either the in or out bufs. Handle compat conversion for
+ * iovec too.
+ */
+static int bsg_map_data(struct request_queue *q, struct request *rq,
+ __u64 uaddr, __u32 tot_len, __u32 numiov,
+ enum dma_data_direction dir)
+{
+ int ret;
+ void __user *ubuf = (void __user *) (unsigned long) uaddr;
+ struct sg_iovec fastiov[8], *iov = fastiov;
+
+ if (numiov == 0) {
+ ret = blk_rq_map_user(q, rq, ubuf, tot_len);
+ goto out;
+ }
+
+ if (numiov >= ARRAY_SIZE(fastiov)) {
+ iov = kmalloc(numiov * sizeof(*iov), GFP_KERNEL);
+ if (iov == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+
+#ifdef CONFIG_COMPAT
+ if (sizeof(struct compat_iovec) != sizeof(*iov)) {
+ struct compat_iovec __user *compat_iov = ubuf;
+ int i;
+ for (i=0; i<numiov; i++) {
+ compat_uptr_t cbase;
+ compat_size_t clen;
+ if (get_user(cbase, &compat_iov[i].iov_base)
+ || get_user(clen, &compat_iov[i].iov_len)) {
+ ret = -EFAULT;
+ goto outfree;
+ }
+ iov[i].iov_base = compat_ptr(cbase);
+ iov[i].iov_len = clen;
+ }
+ goto map;
+ }
+#endif
+
+ if (copy_from_user(iov, ubuf, numiov * sizeof(*iov))) {
+ ret = -EFAULT;
+ goto outfree;
+ }
+map:
+ ret = blk_rq_map_user_iov(q, rq, iov, numiov, tot_len);
+
+outfree:
+ if (iov != fastiov)
+ kfree(iov);
+
+out:
+ return ret;
+}
+
+/*
* map sg_io_v4 to a request.
*/
static struct request *
@@ -288,12 +348,12 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr)
request_queue_t *q = bd->queue;
struct request *rq;
int ret, rw = 0; /* shut up gcc */
- unsigned int dxfer_len;
- void *dxferp = NULL;
- dprintk("map hdr %llx/%u %llx/%u\n", (unsigned long long) hdr->dout_xferp,
- hdr->dout_xfer_len, (unsigned long long) hdr->din_xferp,
- hdr->din_xfer_len);
+ dprintk("map hdr %llx/%u/%u %llx/%u/%u\n",
+ (unsigned long long) hdr->dout_xferp, hdr->dout_xfer_len,
+ hdr->dout_iovec_count,
+ (unsigned long long) hdr->din_xferp, hdr->din_xfer_len,
+ hdr->din_iovec_count);
ret = bsg_validate_sgv4_hdr(q, hdr, &rw);
if (ret)
@@ -305,29 +365,28 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr)
rq = blk_get_request(q, rw, GFP_KERNEL);
ret = blk_fill_sgv4_hdr_rq(q, rq, hdr, test_bit(BSG_F_WRITE_PERM,
&bd->flags));
- if (ret) {
- blk_put_request(rq);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto errout;
if (hdr->dout_xfer_len) {
- dxfer_len = hdr->dout_xfer_len;
- dxferp = (void*)(unsigned long)hdr->dout_xferp;
+ ret = bsg_map_data(q, rq, hdr->dout_xferp, hdr->dout_xfer_len,
+ hdr->dout_iovec_count, DMA_TO_DEVICE);
+ if (ret)
+ goto errout;
} else if (hdr->din_xfer_len) {
- dxfer_len = hdr->din_xfer_len;
- dxferp = (void*)(unsigned long)hdr->din_xferp;
- } else
- dxfer_len = 0;
-
- if (dxfer_len) {
- ret = blk_rq_map_user(q, rq, dxferp, dxfer_len);
- if (ret) {
- dprintk("failed map at %d\n", ret);
- blk_put_request(rq);
- rq = ERR_PTR(ret);
- }
+ ret = bsg_map_data(q, rq, hdr->din_xferp, hdr->din_xfer_len,
+ hdr->din_iovec_count, DMA_FROM_DEVICE);
+ if (ret)
+ goto errout;
}
+ goto out;
+
+errout:
+ blk_put_request(rq);
+ rq = ERR_PTR(ret);
+
+out:
return rq;
}
diff --git a/include/linux/bsg.h b/include/linux/bsg.h
index 51152cb..6bd7311 100644
--- a/include/linux/bsg.h
+++ b/include/linux/bsg.h
@@ -16,6 +16,8 @@ struct sg_io_v4 {
__u64 response; /* [i], [*o] {SCSI: (auto)sense data} */
/* "din_" for data in (from device); "dout_" for data out (to device) */
+ __u32 dout_iovec_count; /* [i] =0 -> "flat" data transfer */
+ __u32 din_iovec_count; /* [i] >0 -> dxfer is struct sg_iovec * */
__u32 dout_xfer_len; /* [i] bytes to be transferred to device */
__u32 din_xfer_len; /* [i] bytes to be transferred from device */
__u64 dout_xferp; /* [i], [*i] */
--
1.5.0.3
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
