Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Rusty Russell]

Asias He <asias@redhat.com> writes:
> scsi.c includes vhost.c which uses memcpy_fromiovec.
>
> This patch fixes this build failure.
>
>    From Randy Dunlap:
>    '''
>    on x86_64:
>
>    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
>
>    It needs to depend on NET since net/core/ provides that function.
>    '''

Proper fix please.

Though I can't see why you thought this was a good idea.  Nonetheless, I
shan't highlight why: I have far too much respect for your intellects
and abilities.

No, don't thank me!
Rusty.

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Nicholas A. Bellinger]

On Wed, 2013-05-15 at 14:47 +0930, Rusty Russell wrote:
> Asias He <asias@redhat.com> writes:
> > scsi.c includes vhost.c which uses memcpy_fromiovec.
> >
> > This patch fixes this build failure.
> >
> >    From Randy Dunlap:
> >    '''
> >    on x86_64:
> >
> >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> >
> >    It needs to depend on NET since net/core/ provides that function.
> >    '''
> 
> Proper fix please.
> 
> Though I can't see why you thought this was a good idea.  Nonetheless, I
> shan't highlight why: I have far too much respect for your intellects
> and abilities.
> 
> No, don't thank me!

Hi Rusty & Asias,

I assume you mean something like the following patch to allow kbuild to
work when VHOST_NET + VHOST_SCSI are both enabled and sharing vhost.o,
yes..?

Also included is dropping the now unnecessary vhost.c include, and
allowing vhost_work_flush() to be accessed externally as scsi.c
currently requires.

MST, care to pick this up..?

--nab

diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index 8b9226d..016387f 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -1,3 +1,6 @@
+config VHOST
+       tristate
+
 config VHOST_NET
        tristate "Host kernel accelerator for virtio net"
        depends on NET && EVENTFD && (TUN || !TUN) && (MACVTAP || !MACVTAP)
@@ -12,7 +15,7 @@ config VHOST_NET
 
 config VHOST_SCSI
        tristate "VHOST_SCSI TCM fabric driver"
-       depends on TARGET_CORE && EVENTFD && m
+       depends on NET && EVENTFD && TARGET_CORE
        select VHOST_RING
        default n
        ---help---
diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
index 654e9afb..e5b5f0b 100644
--- a/drivers/vhost/Makefile
+++ b/drivers/vhost/Makefile
@@ -1,7 +1,9 @@
+obj-$(CONFIG_VHOST) += vhost.o
+
 obj-$(CONFIG_VHOST_NET) += vhost_net.o
-vhost_net-y := vhost.o net.o
+vhost_net-objs := net.o
 
 obj-$(CONFIG_VHOST_SCSI) += vhost_scsi.o
-vhost_scsi-y := scsi.o
+vhost_scsi-objs := scsi.o
 
 obj-$(CONFIG_VHOST_RING) += vringh.o
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 7014202..b5836a2 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -49,7 +49,6 @@
 #include <linux/llist.h>
 #include <linux/bitmap.h>
 
-#include "vhost.c"
 #include "vhost.h"
 
 #define TCM_VHOST_VERSION  "v0.1"
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index beee7f5..8cd1562 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -123,7 +123,7 @@ static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
        return left <= 0;
 }
 
-static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
+void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
 {
        unsigned seq;
        int flushing;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index a7ad635..50ee396 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -44,6 +44,7 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
                     unsigned long mask, struct vhost_dev *dev);
 int vhost_poll_start(struct vhost_poll *poll, struct file *file);
 void vhost_poll_stop(struct vhost_poll *poll);
+void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work);
 void vhost_poll_flush(struct vhost_poll *poll);
 void vhost_poll_queue(struct vhost_poll *poll);

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Rusty Russell]

"Nicholas A. Bellinger" <nab@linux-iscsi.org> writes:
> On Wed, 2013-05-15 at 14:47 +0930, Rusty Russell wrote:
>> Asias He <asias@redhat.com> writes:
>> > scsi.c includes vhost.c which uses memcpy_fromiovec.
>> >
>> > This patch fixes this build failure.
>> >
>> >    From Randy Dunlap:
>> >    '''
>> >    on x86_64:
>> >
>> >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
>> >
>> >    It needs to depend on NET since net/core/ provides that function.
>> >    '''
>> 
>> Proper fix please.
>> 
>> Though I can't see why you thought this was a good idea.  Nonetheless, I
>> shan't highlight why: I have far too much respect for your intellects
>> and abilities.
>> 
>> No, don't thank me!
>
> Hi Rusty & Asias,
>
> I assume you mean something like the following patch to allow kbuild to
> work when VHOST_NET + VHOST_SCSI are both enabled and sharing vhost.o,
> yes..?

No, that's a separate issue.

memcpy_fromiovec() has nothing to do with networking: that was just the
first user.  Note that crypto/algif_skcipher.c also uses it.  The
obvious answer is to move it into lib/.

OTOH making vhost_scsi depend on CONFIG_NET is breathtakingly lazy.  I
expect better from experienced kernel hackers :(

Rusty.

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Asias He]

On Wed, May 15, 2013 at 03:37:30PM -0700, Nicholas A. Bellinger wrote:
> On Wed, 2013-05-15 at 14:47 +0930, Rusty Russell wrote:
> > Asias He <asias@redhat.com> writes:
> > > scsi.c includes vhost.c which uses memcpy_fromiovec.
> > >
> > > This patch fixes this build failure.
> > >
> > >    From Randy Dunlap:
> > >    '''
> > >    on x86_64:
> > >
> > >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> > >
> > >    It needs to depend on NET since net/core/ provides that function.
> > >    '''
> > 
> > Proper fix please.
> > 
> > Though I can't see why you thought this was a good idea.  Nonetheless, I
> > shan't highlight why: I have far too much respect for your intellects
> > and abilities.
> > 
> > No, don't thank me!
> 
> Hi Rusty & Asias,
> 
> I assume you mean something like the following patch to allow kbuild to
> work when VHOST_NET + VHOST_SCSI are both enabled and sharing vhost.o,
> yes..?
> 
> Also included is dropping the now unnecessary vhost.c include, and
> allowing vhost_work_flush() to be accessed externally as scsi.c
> currently requires.
> 
> MST, care to pick this up..?
> 
> --nab


Couple of days ago, I have separated the vhost.ko. 

'vhost: Make vhost a separate module'

http://www.spinics.net/lists/kvm/msg90825.html

MST wanted to queue it up for 3.11. 

> 
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index 8b9226d..016387f 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -1,3 +1,6 @@
> +config VHOST
> +       tristate
> +
>  config VHOST_NET
>         tristate "Host kernel accelerator for virtio net"
>         depends on NET && EVENTFD && (TUN || !TUN) && (MACVTAP || !MACVTAP)
> @@ -12,7 +15,7 @@ config VHOST_NET
>  
>  config VHOST_SCSI
>         tristate "VHOST_SCSI TCM fabric driver"
> -       depends on TARGET_CORE && EVENTFD && m
> +       depends on NET && EVENTFD && TARGET_CORE
>         select VHOST_RING
>         default n
>         ---help---
> diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
> index 654e9afb..e5b5f0b 100644
> --- a/drivers/vhost/Makefile
> +++ b/drivers/vhost/Makefile
> @@ -1,7 +1,9 @@
> +obj-$(CONFIG_VHOST) += vhost.o
> +
>  obj-$(CONFIG_VHOST_NET) += vhost_net.o
> -vhost_net-y := vhost.o net.o
> +vhost_net-objs := net.o
>  
>  obj-$(CONFIG_VHOST_SCSI) += vhost_scsi.o
> -vhost_scsi-y := scsi.o
> +vhost_scsi-objs := scsi.o
>  
>  obj-$(CONFIG_VHOST_RING) += vringh.o
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 7014202..b5836a2 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -49,7 +49,6 @@
>  #include <linux/llist.h>
>  #include <linux/bitmap.h>
>  
> -#include "vhost.c"
>  #include "vhost.h"
>  
>  #define TCM_VHOST_VERSION  "v0.1"
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index beee7f5..8cd1562 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -123,7 +123,7 @@ static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
>         return left <= 0;
>  }
>  
> -static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
> +void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
>  {
>         unsigned seq;
>         int flushing;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index a7ad635..50ee396 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -44,6 +44,7 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
>                      unsigned long mask, struct vhost_dev *dev);
>  int vhost_poll_start(struct vhost_poll *poll, struct file *file);
>  void vhost_poll_stop(struct vhost_poll *poll);
> +void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work);
>  void vhost_poll_flush(struct vhost_poll *poll);
>  void vhost_poll_queue(struct vhost_poll *poll);
> 
> 

-- 
Asias

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Asias He]

On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
> Asias He <asias@redhat.com> writes:
> > scsi.c includes vhost.c which uses memcpy_fromiovec.
> >
> > This patch fixes this build failure.
> >
> >    From Randy Dunlap:
> >    '''
> >    on x86_64:
> >
> >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> >
> >    It needs to depend on NET since net/core/ provides that function.
> >    '''
> 
> Proper fix please.

--verbose please ;-)

Making VHOST_SCSI depends on NET looks weird but this is because vhost
core depends on it. A bunch of patches are cleaning this up. Since MST
wanted do the vhost.ko split up in 3.11, plus your WIP vringh work, so I
wanted the fix for 3.10 as minimum as possible.

Other users are using memcpy_fromiovec and friends outside net. It seems
a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
which also depends on NET for it.

> Though I can't see why you thought this was a good idea.  Nonetheless, I
> shan't highlight why: I have far too much respect for your intellects
> and abilities.
> 
> No, don't thank me!

Interesting.

> Rusty.

-- 
Asias

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Asias He]

On Thu, May 16, 2013 at 09:05:38AM +0930, Rusty Russell wrote:
> "Nicholas A. Bellinger" <nab@linux-iscsi.org> writes:
> > On Wed, 2013-05-15 at 14:47 +0930, Rusty Russell wrote:
> >> Asias He <asias@redhat.com> writes:
> >> > scsi.c includes vhost.c which uses memcpy_fromiovec.
> >> >
> >> > This patch fixes this build failure.
> >> >
> >> >    From Randy Dunlap:
> >> >    '''
> >> >    on x86_64:
> >> >
> >> >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> >> >
> >> >    It needs to depend on NET since net/core/ provides that function.
> >> >    '''
> >> 
> >> Proper fix please.
> >> 
> >> Though I can't see why you thought this was a good idea.  Nonetheless, I
> >> shan't highlight why: I have far too much respect for your intellects
> >> and abilities.
> >> 
> >> No, don't thank me!
> >
> > Hi Rusty & Asias,
> >
> > I assume you mean something like the following patch to allow kbuild to
> > work when VHOST_NET + VHOST_SCSI are both enabled and sharing vhost.o,
> > yes..?
> 
> No, that's a separate issue.
> 
> memcpy_fromiovec() has nothing to do with networking: that was just the
> first user.  Note that crypto/algif_skcipher.c also uses it.  The
> obvious answer is to move it into lib/.

That's true. I also want this.

> OTOH making vhost_scsi depend on CONFIG_NET is breathtakingly lazy.  I
> expect better from experienced kernel hackers :(

But do you think moving the memcpy_fromiovec stuff is a 3.10 material?

> Rusty.

-- 
Asias

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[David Miller]

From: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu, 16 May 2013 09:05:38 +0930

> memcpy_fromiovec() has nothing to do with networking: that was just the
> first user.  Note that crypto/algif_skcipher.c also uses it.  The
> obvious answer is to move it into lib/.

+1

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Rusty Russell]

Asias He <asias@redhat.com> writes:
> On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
>> Asias He <asias@redhat.com> writes:
>> > scsi.c includes vhost.c which uses memcpy_fromiovec.
>> >
>> > This patch fixes this build failure.
>> >
>> >    From Randy Dunlap:
>> >    '''
>> >    on x86_64:
>> >
>> >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
>> >
>> >    It needs to depend on NET since net/core/ provides that function.
>> >    '''
>> 
>> Proper fix please.
>
> --verbose please ;-)
>
> Making VHOST_SCSI depends on NET looks weird but this is because vhost
> core depends on it. A bunch of patches are cleaning this up. Since MST
> wanted do the vhost.ko split up in 3.11, plus your WIP vringh work, so I
> wanted the fix for 3.10 as minimum as possible.

If this isn't the only symbol causing the problem, then this should be
mentioned in the changelog.  If it is, it should be fixed: we have
plenty of time for that.

Either way, your commit message or the commit itself needs to justify
it!

> Other users are using memcpy_fromiovec and friends outside net. It seems
> a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
> which also depends on NET for it.
>
>> Though I can't see why you thought this was a good idea.  Nonetheless, I
>> shan't highlight why: I have far too much respect for your intellects
>> and abilities.
>> 
>> No, don't thank me!
>
> Interesting.

Heh... I originally wrote an explanation, then found it a bit insulting:
I knew I didn't need to tell you :)

How's this?

From: Rusty Russell <rusty@rustcorp.com.au>
Subject: Hoist memcpy_fromiovec into lib/

ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!

That function is only present with CONFIG_NET.  Turns out that
crypto/algif_skcipher.c also uses that outside net, but it actually
needs sockets anyway.

socket.h already include uio.h, so no callers need updating.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 428c37a..7266775 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -305,7 +305,6 @@ struct ucred {
 
 extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct ucred *ucred);
 
-extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
 extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
 			       int offset, int len);
 extern int csum_partial_copy_fromiovecend(unsigned char *kdata, 
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 629aaf5..21628d3 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
 }
 
 unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
 #endif
diff --git a/lib/Makefile b/lib/Makefile
index e9c52e1..2377211 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -9,7 +9,7 @@ endif
 
 lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
-	 idr.o int_sqrt.o extable.o \
+	 idr.o int_sqrt.o extable.o iovec.o \
 	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
 	 proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
 	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
diff --git a/lib/iovec.c b/lib/iovec.c
new file mode 100644
index 0000000..632c5ea
--- /dev/null
+++ b/lib/iovec.c
@@ -0,0 +1,29 @@
+#include <linux/uaccess.h>
+#include <linux/export.h>
+#include <linux/uio.h>
+
+/*
+ *	Copy iovec to kernel. Returns -EFAULT on error.
+ *
+ *	Note: this modifies the original iovec.
+ */
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
+{
+	while (len > 0) {
+		if (iov->iov_len) {
+			int copy = min_t(unsigned int, len, iov->iov_len);
+			if (copy_from_user(kdata, iov->iov_base, copy))
+				return -EFAULT;
+			len -= copy;
+			kdata += copy;
+			iov->iov_base += copy;
+			iov->iov_len -= copy;
+		}
+		iov++;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(memcpy_fromiovec);
+
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 7e7aeb0..d81257f 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -125,31 +125,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
 EXPORT_SYMBOL(memcpy_toiovecend);
 
 /*
- *	Copy iovec to kernel. Returns -EFAULT on error.
- *
- *	Note: this modifies the original iovec.
- */
-
-int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
-{
-	while (len > 0) {
-		if (iov->iov_len) {
-			int copy = min_t(unsigned int, len, iov->iov_len);
-			if (copy_from_user(kdata, iov->iov_base, copy))
-				return -EFAULT;
-			len -= copy;
-			kdata += copy;
-			iov->iov_base += copy;
-			iov->iov_len -= copy;
-		}
-		iov++;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(memcpy_fromiovec);
-
-/*
  *	Copy iovec from kernel. Returns -EFAULT on error.
  */
 

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Joe Perches]

On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
> Asias He <asias@redhat.com> writes:
> > On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
[]
> > Other users are using memcpy_fromiovec and friends outside net. It seems
> > a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
> > which also depends on NET for it.

[]
> Subject: Hoist memcpy_fromiovec into lib/

You'll need the "friends" memcpy_toiovec too.

$ git grep -E \bmemcpy\w+iovec\w*"
crypto/algif_hash.c:    err = memcpy_toiovec(msg->msg_iov, ctx->result, len);
crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg)) +
crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg + i)),
drivers/dma/iovlock.c:#include <net/tcp.h> /* for memcpy_toiovec */
drivers/dma/iovlock.c:          return memcpy_toiovec(iov, kdata, len);
drivers/dma/iovlock.c:          err = memcpy_toiovec(iov, vaddr + offset, len);
drivers/isdn/mISDN/socket.c:    if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_fromiovec((u8 *)va + page_o
drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_toiovec(iov, (u8 *)va + pag

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Asias He]

On Thu, May 16, 2013 at 01:04:58PM +0930, Rusty Russell wrote:
> Asias He <asias@redhat.com> writes:
> > On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
> >> Asias He <asias@redhat.com> writes:
> >> > scsi.c includes vhost.c which uses memcpy_fromiovec.
> >> >
> >> > This patch fixes this build failure.
> >> >
> >> >    From Randy Dunlap:
> >> >    '''
> >> >    on x86_64:
> >> >
> >> >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> >> >
> >> >    It needs to depend on NET since net/core/ provides that function.
> >> >    '''
> >> 
> >> Proper fix please.
> >
> > --verbose please ;-)
> >
> > Making VHOST_SCSI depends on NET looks weird but this is because vhost
> > core depends on it. A bunch of patches are cleaning this up. Since MST
> > wanted do the vhost.ko split up in 3.11, plus your WIP vringh work, so I
> > wanted the fix for 3.10 as minimum as possible.
> 
> If this isn't the only symbol causing the problem, then this should be
> mentioned in the changelog.  If it is, it should be fixed: we have
> plenty of time for that.
>
> Either way, your commit message or the commit itself needs to justify
> it!

memcpy_fromiovec is the only one causing the problem.

> 
> > Other users are using memcpy_fromiovec and friends outside net. It seems
> > a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
> > which also depends on NET for it.
> >
> >> Though I can't see why you thought this was a good idea.  Nonetheless, I
> >> shan't highlight why: I have far too much respect for your intellects
> >> and abilities.
> >> 
> >> No, don't thank me!
> >
> > Interesting.
> 
> Heh... I originally wrote an explanation, then found it a bit insulting:
> I knew I didn't need to tell you :)

;-)

> How's this?

Looks good and the commit log is more informative. 

The

   memcpy_toiovec
   memcpy_toiovecend
   memcpy_fromiovec
   memcpy_fromiovecend

are all not net specific. 

How about move them all to lib/ ?

Also need to make sure all the callers have uio.h included.  e.g.
drivers/dma/iovlock.c

> From: Rusty Russell <rusty@rustcorp.com.au>
> Subject: Hoist memcpy_fromiovec into lib/
> 
> ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> 
> That function is only present with CONFIG_NET.  Turns out that
> crypto/algif_skcipher.c also uses that outside net, but it actually
> needs sockets anyway.
> 
> socket.h already include uio.h, so no callers need updating.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 428c37a..7266775 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -305,7 +305,6 @@ struct ucred {
>  
>  extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct ucred *ucred);
>  
> -extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			       int offset, int len);
>  extern int csum_partial_copy_fromiovecend(unsigned char *kdata, 
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 629aaf5..21628d3 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
>  }
>  
>  unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  #endif
> diff --git a/lib/Makefile b/lib/Makefile
> index e9c52e1..2377211 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -9,7 +9,7 @@ endif
>  
>  lib-y := ctype.o string.o vsprintf.o cmdline.o \
>  	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
> -	 idr.o int_sqrt.o extable.o \
> +	 idr.o int_sqrt.o extable.o iovec.o \
>  	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
>  	 proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
>  	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> diff --git a/lib/iovec.c b/lib/iovec.c
> new file mode 100644
> index 0000000..632c5ea
> --- /dev/null
> +++ b/lib/iovec.c
> @@ -0,0 +1,29 @@
> +#include <linux/uaccess.h>
> +#include <linux/export.h>
> +#include <linux/uio.h>
> +
> +/*
> + *	Copy iovec to kernel. Returns -EFAULT on error.
> + *
> + *	Note: this modifies the original iovec.
> + */
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> +{
> +	while (len > 0) {
> +		if (iov->iov_len) {
> +			int copy = min_t(unsigned int, len, iov->iov_len);
> +			if (copy_from_user(kdata, iov->iov_base, copy))
> +				return -EFAULT;
> +			len -= copy;
> +			kdata += copy;
> +			iov->iov_base += copy;
> +			iov->iov_len -= copy;
> +		}
> +		iov++;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(memcpy_fromiovec);
> +
> diff --git a/net/core/iovec.c b/net/core/iovec.c
> index 7e7aeb0..d81257f 100644
> --- a/net/core/iovec.c
> +++ b/net/core/iovec.c
> @@ -125,31 +125,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
>  EXPORT_SYMBOL(memcpy_toiovecend);
>  
>  /*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - *
> - *	Note: this modifies the original iovec.
> - */
> -
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> -{
> -	while (len > 0) {
> -		if (iov->iov_len) {
> -			int copy = min_t(unsigned int, len, iov->iov_len);
> -			if (copy_from_user(kdata, iov->iov_base, copy))
> -				return -EFAULT;
> -			len -= copy;
> -			kdata += copy;
> -			iov->iov_base += copy;
> -			iov->iov_len -= copy;
> -		}
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovec);
> -
> -/*
>   *	Copy iovec from kernel. Returns -EFAULT on error.
>   */
>  

-- 
Asias

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Michael S. Tsirkin]

On Thu, May 16, 2013 at 01:04:58PM +0930, Rusty Russell wrote:
> Asias He <asias@redhat.com> writes:
> > On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
> >> Asias He <asias@redhat.com> writes:
> >> > scsi.c includes vhost.c which uses memcpy_fromiovec.
> >> >
> >> > This patch fixes this build failure.
> >> >
> >> >    From Randy Dunlap:
> >> >    '''
> >> >    on x86_64:
> >> >
> >> >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> >> >
> >> >    It needs to depend on NET since net/core/ provides that function.
> >> >    '''
> >> 
> >> Proper fix please.
> >
> > --verbose please ;-)
> >
> > Making VHOST_SCSI depends on NET looks weird but this is because vhost
> > core depends on it. A bunch of patches are cleaning this up. Since MST
> > wanted do the vhost.ko split up in 3.11, plus your WIP vringh work, so I
> > wanted the fix for 3.10 as minimum as possible.
> 
> If this isn't the only symbol causing the problem, then this should be
> mentioned in the changelog.  If it is, it should be fixed: we have
> plenty of time for that.
> 
> Either way, your commit message or the commit itself needs to justify
> it!
> 
> > Other users are using memcpy_fromiovec and friends outside net. It seems
> > a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
> > which also depends on NET for it.
> >
> >> Though I can't see why you thought this was a good idea.  Nonetheless, I
> >> shan't highlight why: I have far too much respect for your intellects
> >> and abilities.
> >> 
> >> No, don't thank me!
> >
> > Interesting.
> 
> Heh... I originally wrote an explanation, then found it a bit insulting:
> I knew I didn't need to tell you :)
> 
> How's this?
> 
> From: Rusty Russell <rusty@rustcorp.com.au>
> Subject: Hoist memcpy_fromiovec into lib/
> 
> ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> 
> That function is only present with CONFIG_NET.  Turns out that
> crypto/algif_skcipher.c also uses that outside net, but it actually
> needs sockets anyway.
> 
> socket.h already include uio.h, so no callers need updating.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Would you like me to merge this through the vhost tree?
If I do I can drop #include "linux/socket.h" from vhost.c
right now.

> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 428c37a..7266775 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -305,7 +305,6 @@ struct ucred {
>  
>  extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct ucred *ucred);
>  
> -extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			       int offset, int len);
>  extern int csum_partial_copy_fromiovecend(unsigned char *kdata, 
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 629aaf5..21628d3 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
>  }
>  
>  unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  #endif
> diff --git a/lib/Makefile b/lib/Makefile
> index e9c52e1..2377211 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -9,7 +9,7 @@ endif
>  
>  lib-y := ctype.o string.o vsprintf.o cmdline.o \
>  	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
> -	 idr.o int_sqrt.o extable.o \
> +	 idr.o int_sqrt.o extable.o iovec.o \
>  	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
>  	 proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
>  	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> diff --git a/lib/iovec.c b/lib/iovec.c
> new file mode 100644
> index 0000000..632c5ea
> --- /dev/null
> +++ b/lib/iovec.c
> @@ -0,0 +1,29 @@
> +#include <linux/uaccess.h>
> +#include <linux/export.h>
> +#include <linux/uio.h>
> +
> +/*
> + *	Copy iovec to kernel. Returns -EFAULT on error.
> + *
> + *	Note: this modifies the original iovec.
> + */
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> +{
> +	while (len > 0) {
> +		if (iov->iov_len) {
> +			int copy = min_t(unsigned int, len, iov->iov_len);
> +			if (copy_from_user(kdata, iov->iov_base, copy))
> +				return -EFAULT;
> +			len -= copy;
> +			kdata += copy;
> +			iov->iov_base += copy;
> +			iov->iov_len -= copy;
> +		}
> +		iov++;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(memcpy_fromiovec);
> +
> diff --git a/net/core/iovec.c b/net/core/iovec.c
> index 7e7aeb0..d81257f 100644
> --- a/net/core/iovec.c
> +++ b/net/core/iovec.c
> @@ -125,31 +125,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
>  EXPORT_SYMBOL(memcpy_toiovecend);
>  
>  /*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - *
> - *	Note: this modifies the original iovec.
> - */
> -
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> -{
> -	while (len > 0) {
> -		if (iov->iov_len) {
> -			int copy = min_t(unsigned int, len, iov->iov_len);
> -			if (copy_from_user(kdata, iov->iov_base, copy))
> -				return -EFAULT;
> -			len -= copy;
> -			kdata += copy;
> -			iov->iov_base += copy;
> -			iov->iov_len -= copy;
> -		}
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovec);
> -
> -/*
>   *	Copy iovec from kernel. Returns -EFAULT on error.
>   */
>  
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Michael S. Tsirkin]

On Wed, May 15, 2013 at 03:37:30PM -0700, Nicholas A. Bellinger wrote:
> On Wed, 2013-05-15 at 14:47 +0930, Rusty Russell wrote:
> > Asias He <asias@redhat.com> writes:
> > > scsi.c includes vhost.c which uses memcpy_fromiovec.
> > >
> > > This patch fixes this build failure.
> > >
> > >    From Randy Dunlap:
> > >    '''
> > >    on x86_64:
> > >
> > >    ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> > >
> > >    It needs to depend on NET since net/core/ provides that function.
> > >    '''
> > 
> > Proper fix please.
> > 
> > Though I can't see why you thought this was a good idea.  Nonetheless, I
> > shan't highlight why: I have far too much respect for your intellects
> > and abilities.
> > 
> > No, don't thank me!
> 
> Hi Rusty & Asias,
> 
> I assume you mean something like the following patch to allow kbuild to
> work when VHOST_NET + VHOST_SCSI are both enabled and sharing vhost.o,
> yes..?
> 
> Also included is dropping the now unnecessary vhost.c include, and
> allowing vhost_work_flush() to be accessed externally as scsi.c
> currently requires.
> 
> MST, care to pick this up..?
> 
> --nab

We'll do this for 3.11, 3.10 is bugfixes only now.

> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index 8b9226d..016387f 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -1,3 +1,6 @@
> +config VHOST
> +       tristate
> +
>  config VHOST_NET
>         tristate "Host kernel accelerator for virtio net"
>         depends on NET && EVENTFD && (TUN || !TUN) && (MACVTAP || !MACVTAP)
> @@ -12,7 +15,7 @@ config VHOST_NET
>  
>  config VHOST_SCSI
>         tristate "VHOST_SCSI TCM fabric driver"
> -       depends on TARGET_CORE && EVENTFD && m
> +       depends on NET && EVENTFD && TARGET_CORE
>         select VHOST_RING
>         default n
>         ---help---
> diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
> index 654e9afb..e5b5f0b 100644
> --- a/drivers/vhost/Makefile
> +++ b/drivers/vhost/Makefile
> @@ -1,7 +1,9 @@
> +obj-$(CONFIG_VHOST) += vhost.o
> +
>  obj-$(CONFIG_VHOST_NET) += vhost_net.o
> -vhost_net-y := vhost.o net.o
> +vhost_net-objs := net.o
>  
>  obj-$(CONFIG_VHOST_SCSI) += vhost_scsi.o
> -vhost_scsi-y := scsi.o
> +vhost_scsi-objs := scsi.o
>  
>  obj-$(CONFIG_VHOST_RING) += vringh.o
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 7014202..b5836a2 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -49,7 +49,6 @@
>  #include <linux/llist.h>
>  #include <linux/bitmap.h>
>  
> -#include "vhost.c"
>  #include "vhost.h"
>  
>  #define TCM_VHOST_VERSION  "v0.1"
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index beee7f5..8cd1562 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -123,7 +123,7 @@ static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
>         return left <= 0;
>  }
>  
> -static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
> +void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
>  {
>         unsigned seq;
>         int flushing;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index a7ad635..50ee396 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -44,6 +44,7 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
>                      unsigned long mask, struct vhost_dev *dev);
>  int vhost_poll_start(struct vhost_poll *poll, struct file *file);
>  void vhost_poll_stop(struct vhost_poll *poll);
> +void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work);
>  void vhost_poll_flush(struct vhost_poll *poll);
>  void vhost_poll_queue(struct vhost_poll *poll);
> 

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Michael S. Tsirkin]

On Wed, May 15, 2013 at 08:10:55PM -0700, David Miller wrote:
> From: Rusty Russell <rusty@rustcorp.com.au>
> Date: Thu, 16 May 2013 09:05:38 +0930
> 
> > memcpy_fromiovec() has nothing to do with networking: that was just the
> > first user.  Note that crypto/algif_skcipher.c also uses it.  The
> > obvious answer is to move it into lib/.
> 
> +1

Rusty sent a patch that does this:
http://patchwork.ozlabs.org/patch/244207/

David, looks like you weren't CC'd.
If you agree could you please Ack that patch and then I can merge it
through the vhost tree?
Or if you prefer merge it directly and I'll sort out the dependencies...

Thanks,

-- 
MST

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[David Miller]

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 16 May 2013 09:46:21 +0300

> On Wed, May 15, 2013 at 08:10:55PM -0700, David Miller wrote:
>> From: Rusty Russell <rusty@rustcorp.com.au>
>> Date: Thu, 16 May 2013 09:05:38 +0930
>> 
>> > memcpy_fromiovec() has nothing to do with networking: that was just the
>> > first user.  Note that crypto/algif_skcipher.c also uses it.  The
>> > obvious answer is to move it into lib/.
>> 
>> +1
> 
> Rusty sent a patch that does this:
> http://patchwork.ozlabs.org/patch/244207/
> 
> David, looks like you weren't CC'd.
> If you agree could you please Ack that patch and then I can merge it
> through the vhost tree?
> Or if you prefer merge it directly and I'll sort out the dependencies...

Acked-by: David S. Miller <davem@davemloft.net>

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Rusty Russell]

Joe Perches <joe@perches.com> writes:
> On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
>> Asias He <asias@redhat.com> writes:
>> > On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
> []
>> > Other users are using memcpy_fromiovec and friends outside net. It seems
>> > a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
>> > which also depends on NET for it.
>
> []
>> Subject: Hoist memcpy_fromiovec into lib/
>
> You'll need the "friends" memcpy_toiovec too.
>
> $ git grep -E \bmemcpy\w+iovec\w*"
> crypto/algif_hash.c:    err = memcpy_toiovec(msg->msg_iov, ctx->result, len);
> crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg)) +
> crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg + i)),
> drivers/dma/iovlock.c:#include <net/tcp.h> /* for memcpy_toiovec */
> drivers/dma/iovlock.c:          return memcpy_toiovec(iov, kdata, len);
> drivers/dma/iovlock.c:          err = memcpy_toiovec(iov, vaddr + offset, len);
> drivers/isdn/mISDN/socket.c:    if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_fromiovec((u8 *)va + page_o
> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_toiovec(iov, (u8 *)va + pag

Fascinating.  These all indirectly depend on NET, so there's no problem
at the moment.  But it is a bit weird...

crypto/algif_hash.c: depends on CRYPTO_USER_API_HASH -> NET
crypto/algif_skcipher.c: depends on CRYPTO_USER_API_SKCIPHER -> NET
drivers/dma/iovlock.c: depends on NET_DMA -> NET
drivers/isdn/mISDN/socket.c: depends on MISDN -> ISDN -> NET
drivers/misc/vmw_vmci/vmci_queue_pair.c: depends on VMCI -> NET

Patch welcome.

Meanwhile, to avoid more bikeshedding I've put the patch I posted with
all acks in my fixes branch.  One cycle through linux-next, then
straight to Linus.

Subject: Hoist memcpy_fromiovec into lib/

ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!

That function is only present with CONFIG_NET.  Turns out that
crypto/algif_skcipher.c also uses that outside net, but it actually
needs sockets anyway.

socket.h already include uio.h, so no callers need updating.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 428c37a..7266775 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -305,7 +305,6 @@ struct ucred {
 
 extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct ucred *ucred);
 
-extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
 extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
 			       int offset, int len);
 extern int csum_partial_copy_fromiovecend(unsigned char *kdata, 
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 629aaf5..21628d3 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
 }
 
 unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
 #endif
diff --git a/lib/Makefile b/lib/Makefile
index e9c52e1..2377211 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -9,7 +9,7 @@ endif
 
 lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
-	 idr.o int_sqrt.o extable.o \
+	 idr.o int_sqrt.o extable.o iovec.o \
 	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
 	 proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
 	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
diff --git a/lib/iovec.c b/lib/iovec.c
new file mode 100644
index 0000000..632c5ea
--- /dev/null
+++ b/lib/iovec.c
@@ -0,0 +1,29 @@
+#include <linux/uaccess.h>
+#include <linux/export.h>
+#include <linux/uio.h>
+
+/*
+ *	Copy iovec to kernel. Returns -EFAULT on error.
+ *
+ *	Note: this modifies the original iovec.
+ */
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
+{
+	while (len > 0) {
+		if (iov->iov_len) {
+			int copy = min_t(unsigned int, len, iov->iov_len);
+			if (copy_from_user(kdata, iov->iov_base, copy))
+				return -EFAULT;
+			len -= copy;
+			kdata += copy;
+			iov->iov_base += copy;
+			iov->iov_len -= copy;
+		}
+		iov++;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(memcpy_fromiovec);
+
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 7e7aeb0..d81257f 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -125,31 +125,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
 EXPORT_SYMBOL(memcpy_toiovecend);
 
 /*
- *	Copy iovec to kernel. Returns -EFAULT on error.
- *
- *	Note: this modifies the original iovec.
- */
-
-int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
-{
-	while (len > 0) {
-		if (iov->iov_len) {
-			int copy = min_t(unsigned int, len, iov->iov_len);
-			if (copy_from_user(kdata, iov->iov_base, copy))
-				return -EFAULT;
-			len -= copy;
-			kdata += copy;
-			iov->iov_base += copy;
-			iov->iov_len -= copy;
-		}
-		iov++;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(memcpy_fromiovec);
-
-/*
  *	Copy iovec from kernel. Returns -EFAULT on error.
  */

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Randy Dunlap]

On 05/16/13 16:42, Rusty Russell wrote:
> Joe Perches <joe@perches.com> writes:
>> On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
>>> Asias He <asias@redhat.com> writes:
>>>> On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
>> []
>>>> Other users are using memcpy_fromiovec and friends outside net. It seems
>>>> a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
>>>> which also depends on NET for it.
>>
>> []
>>> Subject: Hoist memcpy_fromiovec into lib/
>>
>> You'll need the "friends" memcpy_toiovec too.
>>
>> $ git grep -E \bmemcpy\w+iovec\w*"
>> crypto/algif_hash.c:    err = memcpy_toiovec(msg->msg_iov, ctx->result, len);
>> crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg)) +
>> crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg + i)),
>> drivers/dma/iovlock.c:#include <net/tcp.h> /* for memcpy_toiovec */
>> drivers/dma/iovlock.c:          return memcpy_toiovec(iov, kdata, len);
>> drivers/dma/iovlock.c:          err = memcpy_toiovec(iov, vaddr + offset, len);
>> drivers/isdn/mISDN/socket.c:    if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
>> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_fromiovec((u8 *)va + page_o
>> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_toiovec(iov, (u8 *)va + pag
> 
> Fascinating.  These all indirectly depend on NET, so there's no problem
> at the moment.  But it is a bit weird...
> 
> crypto/algif_hash.c: depends on CRYPTO_USER_API_HASH -> NET
> crypto/algif_skcipher.c: depends on CRYPTO_USER_API_SKCIPHER -> NET
> drivers/dma/iovlock.c: depends on NET_DMA -> NET
> drivers/isdn/mISDN/socket.c: depends on MISDN -> ISDN -> NET
> drivers/misc/vmw_vmci/vmci_queue_pair.c: depends on VMCI -> NET
> 
> Patch welcome.
> 
> Meanwhile, to avoid more bikeshedding I've put the patch I posted with
> all acks in my fixes branch.  One cycle through linux-next, then
> straight to Linus.
> 

I agree with whoever suggested that more be moved into /lib.
E.g., drivers/misc/vmw_vmci/Kconfig uses "depends on NET" because the
code there uses both memcpy_toiovec() and memcpy_fromiovec().
See commit ID 6d4f0139d642c45411a47879325891ce2a7c164a.


> Subject: Hoist memcpy_fromiovec into lib/
> 
> ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> 
> That function is only present with CONFIG_NET.  Turns out that
> crypto/algif_skcipher.c also uses that outside net, but it actually
> needs sockets anyway.
> 
> socket.h already include uio.h, so no callers need updating.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Acked-by: David S. Miller <davem@davemloft.net>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 428c37a..7266775 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -305,7 +305,6 @@ struct ucred {
>  
>  extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct ucred *ucred);
>  
> -extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			       int offset, int len);
>  extern int csum_partial_copy_fromiovecend(unsigned char *kdata, 
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 629aaf5..21628d3 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
>  }
>  
>  unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  #endif
> diff --git a/lib/Makefile b/lib/Makefile
> index e9c52e1..2377211 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -9,7 +9,7 @@ endif
>  
>  lib-y := ctype.o string.o vsprintf.o cmdline.o \
>  	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
> -	 idr.o int_sqrt.o extable.o \
> +	 idr.o int_sqrt.o extable.o iovec.o \
>  	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
>  	 proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
>  	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> diff --git a/lib/iovec.c b/lib/iovec.c
> new file mode 100644
> index 0000000..632c5ea
> --- /dev/null
> +++ b/lib/iovec.c
> @@ -0,0 +1,29 @@
> +#include <linux/uaccess.h>
> +#include <linux/export.h>
> +#include <linux/uio.h>
> +
> +/*
> + *	Copy iovec to kernel. Returns -EFAULT on error.
> + *
> + *	Note: this modifies the original iovec.
> + */
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> +{
> +	while (len > 0) {
> +		if (iov->iov_len) {
> +			int copy = min_t(unsigned int, len, iov->iov_len);
> +			if (copy_from_user(kdata, iov->iov_base, copy))
> +				return -EFAULT;
> +			len -= copy;
> +			kdata += copy;
> +			iov->iov_base += copy;
> +			iov->iov_len -= copy;
> +		}
> +		iov++;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(memcpy_fromiovec);
> +
> diff --git a/net/core/iovec.c b/net/core/iovec.c
> index 7e7aeb0..d81257f 100644
> --- a/net/core/iovec.c
> +++ b/net/core/iovec.c
> @@ -125,31 +125,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
>  EXPORT_SYMBOL(memcpy_toiovecend);
>  
>  /*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - *
> - *	Note: this modifies the original iovec.
> - */
> -
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> -{
> -	while (len > 0) {
> -		if (iov->iov_len) {
> -			int copy = min_t(unsigned int, len, iov->iov_len);
> -			if (copy_from_user(kdata, iov->iov_base, copy))
> -				return -EFAULT;
> -			len -= copy;
> -			kdata += copy;
> -			iov->iov_base += copy;
> -			iov->iov_len -= copy;
> -		}
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovec);
> -
> -/*
>   *	Copy iovec from kernel. Returns -EFAULT on error.
>   */
>  
> 


-- 
~Randy

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Rusty Russell]

Randy Dunlap <rdunlap@infradead.org> writes:
> On 05/16/13 16:42, Rusty Russell wrote:
>> Joe Perches <joe@perches.com> writes:
>>> On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
>>>> Asias He <asias@redhat.com> writes:
>>>>> On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
>>> []
>>>>> Other users are using memcpy_fromiovec and friends outside net. It seems
>>>>> a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
>>>>> which also depends on NET for it.
>>>
>>> []
>>>> Subject: Hoist memcpy_fromiovec into lib/
>>>
>>> You'll need the "friends" memcpy_toiovec too.
>>>
>>> $ git grep -E \bmemcpy\w+iovec\w*"
>>> crypto/algif_hash.c:    err = memcpy_toiovec(msg->msg_iov, ctx->result, len);
>>> crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg)) +
>>> crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg + i)),
>>> drivers/dma/iovlock.c:#include <net/tcp.h> /* for memcpy_toiovec */
>>> drivers/dma/iovlock.c:          return memcpy_toiovec(iov, kdata, len);
>>> drivers/dma/iovlock.c:          err = memcpy_toiovec(iov, vaddr + offset, len);
>>> drivers/isdn/mISDN/socket.c:    if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
>>> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_fromiovec((u8 *)va + page_o
>>> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_toiovec(iov, (u8 *)va + pag
>> 
>> Fascinating.  These all indirectly depend on NET, so there's no problem
>> at the moment.  But it is a bit weird...
>> 
>> crypto/algif_hash.c: depends on CRYPTO_USER_API_HASH -> NET
>> crypto/algif_skcipher.c: depends on CRYPTO_USER_API_SKCIPHER -> NET
>> drivers/dma/iovlock.c: depends on NET_DMA -> NET
>> drivers/isdn/mISDN/socket.c: depends on MISDN -> ISDN -> NET
>> drivers/misc/vmw_vmci/vmci_queue_pair.c: depends on VMCI -> NET
>> 
>> Patch welcome.
>> 
>> Meanwhile, to avoid more bikeshedding I've put the patch I posted with
>> all acks in my fixes branch.  One cycle through linux-next, then
>> straight to Linus.
>> 
>
> I agree with whoever suggested that more be moved into /lib.
> E.g., drivers/misc/vmw_vmci/Kconfig uses "depends on NET" because the
> code there uses both memcpy_toiovec() and memcpy_fromiovec().
> See commit ID 6d4f0139d642c45411a47879325891ce2a7c164a.

(CC's trimmed).

You Acked that commit :(

At a glance, the only way to drive the vmw_vmci device is through
net/vmw_vsock/vmci_transport.c, so without NET it's useless?  But let's
keep it neat anyway.  This was compiletested with CONFIG_VMCI,
CONFIG_DMA_ENGINE and !CONFIG_NET.

Thanks,
Rusty.

From: Rusty Russell <rusty@rustcorp.com.au>
Subject: [PATCH] Hoist memcpy_fromiovec/memcpy_toiovec into lib/

ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!

That function is only present with CONFIG_NET.  Turns out that
crypto/algif_skcipher.c also uses that outside net, but it actually
needs sockets anyway.

In addition, commit 6d4f0139d642c45411a47879325891ce2a7c164a added
CONFIG_NET dependency to CONFIG_VMCI for memcpy_toiovec, so hoist
that function and revert that commit too.

socket.h already include uio.h, so no callers *need* updating,
though I update the obvious ones.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/drivers/dma/iovlock.c b/drivers/dma/iovlock.c
index bb48a57..6943deb66 100644
--- a/drivers/dma/iovlock.c
+++ b/drivers/dma/iovlock.c
@@ -28,7 +28,7 @@
 #include <linux/dmaengine.h>
 #include <linux/pagemap.h>
 #include <linux/slab.h>
-#include <net/tcp.h> /* for memcpy_toiovec */
+#include <linux/uio.h> /* for memcpy_toiovec */
 #include <asm/io.h>
 #include <asm/uaccess.h>
 
diff --git a/drivers/misc/vmw_vmci/Kconfig b/drivers/misc/vmw_vmci/Kconfig
index ea98f7e..39c2eca 100644
--- a/drivers/misc/vmw_vmci/Kconfig
+++ b/drivers/misc/vmw_vmci/Kconfig
@@ -4,7 +4,7 @@
 
 config VMWARE_VMCI
 	tristate "VMware VMCI Driver"
-	depends on X86 && PCI && NET
+	depends on X86 && PCI
 	help
 	  This is VMware's Virtual Machine Communication Interface.  It enables
 	  high-speed communication between host and guest in a virtual
diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index d94245d..8ff2e5e 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -23,7 +23,7 @@
 #include <linux/pagemap.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
-#include <linux/socket.h>
+#include <linux/uio.h>
 #include <linux/wait.h>
 #include <linux/vmalloc.h>
 
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 428c37a..33bf2df 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -305,7 +305,6 @@ struct ucred {
 
 extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct ucred *ucred);
 
-extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
 extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
 			       int offset, int len);
 extern int csum_partial_copy_fromiovecend(unsigned char *kdata, 
@@ -314,7 +313,6 @@ extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
 					  unsigned int len, __wsum *csump);
 
 extern int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode);
-extern int memcpy_toiovec(struct iovec *v, unsigned char *kdata, int len);
 extern int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
 			     int offset, int len);
 extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 629aaf5..c55ce24 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -35,4 +35,7 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
 }
 
 unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
+int memcpy_toiovec(struct iovec *iov, unsigned char *kdata, int len);
 #endif
diff --git a/lib/Makefile b/lib/Makefile
index e9c52e1..2377211 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -9,7 +9,7 @@ endif
 
 lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
-	 idr.o int_sqrt.o extable.o \
+	 idr.o int_sqrt.o extable.o iovec.o \
 	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
 	 proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
 	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
diff --git a/lib/iovec.c b/lib/iovec.c
new file mode 100644
index 0000000..454baa8
--- /dev/null
+++ b/lib/iovec.c
@@ -0,0 +1,53 @@
+#include <linux/uaccess.h>
+#include <linux/export.h>
+#include <linux/uio.h>
+
+/*
+ *	Copy iovec to kernel. Returns -EFAULT on error.
+ *
+ *	Note: this modifies the original iovec.
+ */
+
+int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
+{
+	while (len > 0) {
+		if (iov->iov_len) {
+			int copy = min_t(unsigned int, len, iov->iov_len);
+			if (copy_from_user(kdata, iov->iov_base, copy))
+				return -EFAULT;
+			len -= copy;
+			kdata += copy;
+			iov->iov_base += copy;
+			iov->iov_len -= copy;
+		}
+		iov++;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(memcpy_fromiovec);
+
+/*
+ *	Copy kernel to iovec. Returns -EFAULT on error.
+ *
+ *	Note: this modifies the original iovec.
+ */
+
+int memcpy_toiovec(struct iovec *iov, unsigned char *kdata, int len)
+{
+	while (len > 0) {
+		if (iov->iov_len) {
+			int copy = min_t(unsigned int, iov->iov_len, len);
+			if (copy_to_user(iov->iov_base, kdata, copy))
+				return -EFAULT;
+			kdata += copy;
+			len -= copy;
+			iov->iov_len -= copy;
+			iov->iov_base += copy;
+		}
+		iov++;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(memcpy_toiovec);
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 7e7aeb0..de178e4 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -75,31 +75,6 @@ int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *a
 
 /*
  *	Copy kernel to iovec. Returns -EFAULT on error.
- *
- *	Note: this modifies the original iovec.
- */
-
-int memcpy_toiovec(struct iovec *iov, unsigned char *kdata, int len)
-{
-	while (len > 0) {
-		if (iov->iov_len) {
-			int copy = min_t(unsigned int, iov->iov_len, len);
-			if (copy_to_user(iov->iov_base, kdata, copy))
-				return -EFAULT;
-			kdata += copy;
-			len -= copy;
-			iov->iov_len -= copy;
-			iov->iov_base += copy;
-		}
-		iov++;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(memcpy_toiovec);
-
-/*
- *	Copy kernel to iovec. Returns -EFAULT on error.
  */
 
 int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
@@ -125,31 +100,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
 EXPORT_SYMBOL(memcpy_toiovecend);
 
 /*
- *	Copy iovec to kernel. Returns -EFAULT on error.
- *
- *	Note: this modifies the original iovec.
- */
-
-int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
-{
-	while (len > 0) {
-		if (iov->iov_len) {
-			int copy = min_t(unsigned int, len, iov->iov_len);
-			if (copy_from_user(kdata, iov->iov_base, copy))
-				return -EFAULT;
-			len -= copy;
-			kdata += copy;
-			iov->iov_base += copy;
-			iov->iov_len -= copy;
-		}
-		iov++;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(memcpy_fromiovec);
-
-/*
  *	Copy iovec from kernel. Returns -EFAULT on error.
  */
 

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Asias He]

On Fri, May 17, 2013 at 04:25:49PM +0930, Rusty Russell wrote:
> Randy Dunlap <rdunlap@infradead.org> writes:
> > On 05/16/13 16:42, Rusty Russell wrote:
> >> Joe Perches <joe@perches.com> writes:
> >>> On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
> >>>> Asias He <asias@redhat.com> writes:
> >>>>> On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
> >>> []
> >>>>> Other users are using memcpy_fromiovec and friends outside net. It seems
> >>>>> a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
> >>>>> which also depends on NET for it.
> >>>
> >>> []
> >>>> Subject: Hoist memcpy_fromiovec into lib/
> >>>
> >>> You'll need the "friends" memcpy_toiovec too.
> >>>
> >>> $ git grep -E \bmemcpy\w+iovec\w*"
> >>> crypto/algif_hash.c:    err = memcpy_toiovec(msg->msg_iov, ctx->result, len);
> >>> crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg)) +
> >>> crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg + i)),
> >>> drivers/dma/iovlock.c:#include <net/tcp.h> /* for memcpy_toiovec */
> >>> drivers/dma/iovlock.c:          return memcpy_toiovec(iov, kdata, len);
> >>> drivers/dma/iovlock.c:          err = memcpy_toiovec(iov, vaddr + offset, len);
> >>> drivers/isdn/mISDN/socket.c:    if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
> >>> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_fromiovec((u8 *)va + page_o
> >>> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_toiovec(iov, (u8 *)va + pag
> >> 
> >> Fascinating.  These all indirectly depend on NET, so there's no problem
> >> at the moment.  But it is a bit weird...
> >> 
> >> crypto/algif_hash.c: depends on CRYPTO_USER_API_HASH -> NET
> >> crypto/algif_skcipher.c: depends on CRYPTO_USER_API_SKCIPHER -> NET
> >> drivers/dma/iovlock.c: depends on NET_DMA -> NET
> >> drivers/isdn/mISDN/socket.c: depends on MISDN -> ISDN -> NET
> >> drivers/misc/vmw_vmci/vmci_queue_pair.c: depends on VMCI -> NET
> >> 
> >> Patch welcome.
> >> 
> >> Meanwhile, to avoid more bikeshedding I've put the patch I posted with
> >> all acks in my fixes branch.  One cycle through linux-next, then
> >> straight to Linus.
> >> 
> >
> > I agree with whoever suggested that more be moved into /lib.
> > E.g., drivers/misc/vmw_vmci/Kconfig uses "depends on NET" because the
> > code there uses both memcpy_toiovec() and memcpy_fromiovec().
> > See commit ID 6d4f0139d642c45411a47879325891ce2a7c164a.
> 
> (CC's trimmed).
> 
> You Acked that commit :(
> 
> At a glance, the only way to drive the vmw_vmci device is through
> net/vmw_vsock/vmci_transport.c, so without NET it's useless?  But let's
> keep it neat anyway.  This was compiletested with CONFIG_VMCI,
> CONFIG_DMA_ENGINE and !CONFIG_NET.
> 
> Thanks,
> Rusty.
> 
> From: Rusty Russell <rusty@rustcorp.com.au>
> Subject: [PATCH] Hoist memcpy_fromiovec/memcpy_toiovec into lib/
> 
> ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> 
> That function is only present with CONFIG_NET.  Turns out that
> crypto/algif_skcipher.c also uses that outside net, but it actually
> needs sockets anyway.
> 
> In addition, commit 6d4f0139d642c45411a47879325891ce2a7c164a added
> CONFIG_NET dependency to CONFIG_VMCI for memcpy_toiovec, so hoist
> that function and revert that commit too.
> 
> socket.h already include uio.h, so no callers *need* updating,
> though I update the obvious ones.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Acked-by: David S. Miller <davem@davemloft.net>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Acked-by: Asias He <asias@redhat.com>

> diff --git a/drivers/dma/iovlock.c b/drivers/dma/iovlock.c
> index bb48a57..6943deb66 100644
> --- a/drivers/dma/iovlock.c
> +++ b/drivers/dma/iovlock.c
> @@ -28,7 +28,7 @@
>  #include <linux/dmaengine.h>
>  #include <linux/pagemap.h>
>  #include <linux/slab.h>
> -#include <net/tcp.h> /* for memcpy_toiovec */
> +#include <linux/uio.h> /* for memcpy_toiovec */
>  #include <asm/io.h>
>  #include <asm/uaccess.h>
>  
> diff --git a/drivers/misc/vmw_vmci/Kconfig b/drivers/misc/vmw_vmci/Kconfig
> index ea98f7e..39c2eca 100644
> --- a/drivers/misc/vmw_vmci/Kconfig
> +++ b/drivers/misc/vmw_vmci/Kconfig
> @@ -4,7 +4,7 @@
>  
>  config VMWARE_VMCI
>  	tristate "VMware VMCI Driver"
> -	depends on X86 && PCI && NET
> +	depends on X86 && PCI
>  	help
>  	  This is VMware's Virtual Machine Communication Interface.  It enables
>  	  high-speed communication between host and guest in a virtual
> diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> index d94245d..8ff2e5e 100644
> --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
> +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> @@ -23,7 +23,7 @@
>  #include <linux/pagemap.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
> -#include <linux/socket.h>
> +#include <linux/uio.h>
>  #include <linux/wait.h>
>  #include <linux/vmalloc.h>
>  
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 428c37a..33bf2df 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -305,7 +305,6 @@ struct ucred {
>  
>  extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct ucred *ucred);
>  
> -extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			       int offset, int len);
>  extern int csum_partial_copy_fromiovecend(unsigned char *kdata, 
> @@ -314,7 +313,6 @@ extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
>  					  unsigned int len, __wsum *csump);
>  
>  extern int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode);
> -extern int memcpy_toiovec(struct iovec *v, unsigned char *kdata, int len);
>  extern int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
>  			     int offset, int len);
>  extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 629aaf5..c55ce24 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -35,4 +35,7 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
>  }
>  
>  unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
> +int memcpy_toiovec(struct iovec *iov, unsigned char *kdata, int len);
>  #endif
> diff --git a/lib/Makefile b/lib/Makefile
> index e9c52e1..2377211 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -9,7 +9,7 @@ endif
>  
>  lib-y := ctype.o string.o vsprintf.o cmdline.o \
>  	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
> -	 idr.o int_sqrt.o extable.o \
> +	 idr.o int_sqrt.o extable.o iovec.o \
>  	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
>  	 proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
>  	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> diff --git a/lib/iovec.c b/lib/iovec.c
> new file mode 100644
> index 0000000..454baa8
> --- /dev/null
> +++ b/lib/iovec.c
> @@ -0,0 +1,53 @@
> +#include <linux/uaccess.h>
> +#include <linux/export.h>
> +#include <linux/uio.h>
> +
> +/*
> + *	Copy iovec to kernel. Returns -EFAULT on error.
> + *
> + *	Note: this modifies the original iovec.
> + */
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> +{
> +	while (len > 0) {
> +		if (iov->iov_len) {
> +			int copy = min_t(unsigned int, len, iov->iov_len);
> +			if (copy_from_user(kdata, iov->iov_base, copy))
> +				return -EFAULT;
> +			len -= copy;
> +			kdata += copy;
> +			iov->iov_base += copy;
> +			iov->iov_len -= copy;
> +		}
> +		iov++;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(memcpy_fromiovec);
> +
> +/*
> + *	Copy kernel to iovec. Returns -EFAULT on error.
> + *
> + *	Note: this modifies the original iovec.
> + */
> +
> +int memcpy_toiovec(struct iovec *iov, unsigned char *kdata, int len)
> +{
> +	while (len > 0) {
> +		if (iov->iov_len) {
> +			int copy = min_t(unsigned int, iov->iov_len, len);
> +			if (copy_to_user(iov->iov_base, kdata, copy))
> +				return -EFAULT;
> +			kdata += copy;
> +			len -= copy;
> +			iov->iov_len -= copy;
> +			iov->iov_base += copy;
> +		}
> +		iov++;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(memcpy_toiovec);
> diff --git a/net/core/iovec.c b/net/core/iovec.c
> index 7e7aeb0..de178e4 100644
> --- a/net/core/iovec.c
> +++ b/net/core/iovec.c
> @@ -75,31 +75,6 @@ int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *a
>  
>  /*
>   *	Copy kernel to iovec. Returns -EFAULT on error.
> - *
> - *	Note: this modifies the original iovec.
> - */
> -
> -int memcpy_toiovec(struct iovec *iov, unsigned char *kdata, int len)
> -{
> -	while (len > 0) {
> -		if (iov->iov_len) {
> -			int copy = min_t(unsigned int, iov->iov_len, len);
> -			if (copy_to_user(iov->iov_base, kdata, copy))
> -				return -EFAULT;
> -			kdata += copy;
> -			len -= copy;
> -			iov->iov_len -= copy;
> -			iov->iov_base += copy;
> -		}
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_toiovec);
> -
> -/*
> - *	Copy kernel to iovec. Returns -EFAULT on error.
>   */
>  
>  int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
> @@ -125,31 +100,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
>  EXPORT_SYMBOL(memcpy_toiovecend);
>  
>  /*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - *
> - *	Note: this modifies the original iovec.
> - */
> -
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> -{
> -	while (len > 0) {
> -		if (iov->iov_len) {
> -			int copy = min_t(unsigned int, len, iov->iov_len);
> -			if (copy_from_user(kdata, iov->iov_base, copy))
> -				return -EFAULT;
> -			len -= copy;
> -			kdata += copy;
> -			iov->iov_base += copy;
> -			iov->iov_len -= copy;
> -		}
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovec);
> -
> -/*
>   *	Copy iovec from kernel. Returns -EFAULT on error.
>   */
>  

-- 
Asias

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Dmitry Torokhov]

On Monday, May 20, 2013 10:07:14 AM Asias He wrote:
> On Fri, May 17, 2013 at 04:25:49PM +0930, Rusty Russell wrote:
> > Randy Dunlap <rdunlap@infradead.org> writes:
> > > On 05/16/13 16:42, Rusty Russell wrote:
> > >> Joe Perches <joe@perches.com> writes:
> > >>> On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
> > >>>> Asias He <asias@redhat.com> writes:
> > >>>>> On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
> > >>> []
> > >>> 
> > >>>>> Other users are using memcpy_fromiovec and friends outside net. It
> > >>>>> seems
> > >>>>> a good idea to put it in a util library. e.g. 
> > >>>>> crypto/algif_skcipher.c
> > >>>>> which also depends on NET for it.
> > >>> 
> > >>> []
> > >>> 
> > >>>> Subject: Hoist memcpy_fromiovec into lib/
> > >>> 
> > >>> You'll need the "friends" memcpy_toiovec too.
> > >>> 
> > >>> $ git grep -E \bmemcpy\w+iovec\w*"
> > >>> crypto/algif_hash.c:    err = memcpy_toiovec(msg->msg_iov,
> > >>> ctx->result, len); crypto/algif_skcipher.c:                       
> > >>> err = memcpy_fromiovec(page_address(sg_page(sg)) +
> > >>> crypto/algif_skcipher.c:                        err =
> > >>> memcpy_fromiovec(page_address(sg_page(sg + i)),
> > >>> drivers/dma/iovlock.c:#include <net/tcp.h> /* for memcpy_toiovec */
> > >>> drivers/dma/iovlock.c:          return memcpy_toiovec(iov, kdata,
> > >>> len); drivers/dma/iovlock.c:          err = memcpy_toiovec(iov, vaddr
> > >>> + offset, len); drivers/isdn/mISDN/socket.c:    if
> > >>> (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
> > >>> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err =
> > >>> memcpy_fromiovec((u8 *)va + page_o
> > >>> drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err =
> > >>> memcpy_toiovec(iov, (u8 *)va + pag> >> 
> > >> Fascinating.  These all indirectly depend on NET, so there's no problem
> > >> at the moment.  But it is a bit weird...
> > >> 
> > >> crypto/algif_hash.c: depends on CRYPTO_USER_API_HASH -> NET
> > >> crypto/algif_skcipher.c: depends on CRYPTO_USER_API_SKCIPHER -> NET
> > >> drivers/dma/iovlock.c: depends on NET_DMA -> NET
> > >> drivers/isdn/mISDN/socket.c: depends on MISDN -> ISDN -> NET
> > >> drivers/misc/vmw_vmci/vmci_queue_pair.c: depends on VMCI -> NET
> > >> 
> > >> Patch welcome.
> > >> 
> > >> Meanwhile, to avoid more bikeshedding I've put the patch I posted with
> > >> all acks in my fixes branch.  One cycle through linux-next, then
> > >> straight to Linus.
> > > 
> > > I agree with whoever suggested that more be moved into /lib.
> > > E.g., drivers/misc/vmw_vmci/Kconfig uses "depends on NET" because the
> > > code there uses both memcpy_toiovec() and memcpy_fromiovec().
> > > See commit ID 6d4f0139d642c45411a47879325891ce2a7c164a.
> > 
> > (CC's trimmed).
> > 
> > You Acked that commit :(
> > 
> > At a glance, the only way to drive the vmw_vmci device is through
> > net/vmw_vsock/vmci_transport.c, so without NET it's useless?  But let's
> > keep it neat anyway.  This was compiletested with CONFIG_VMCI,
> > CONFIG_DMA_ENGINE and !CONFIG_NET.
> > 
> > Thanks,
> > Rusty.
> > 
> > From: Rusty Russell <rusty@rustcorp.com.au>
> > Subject: [PATCH] Hoist memcpy_fromiovec/memcpy_toiovec into lib/
> > 
> > ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> > 
> > That function is only present with CONFIG_NET.  Turns out that
> > crypto/algif_skcipher.c also uses that outside net, but it actually
> > needs sockets anyway.
> > 
> > In addition, commit 6d4f0139d642c45411a47879325891ce2a7c164a added
> > CONFIG_NET dependency to CONFIG_VMCI for memcpy_toiovec, so hoist
> > that function and revert that commit too.
> > 
> > socket.h already include uio.h, so no callers *need* updating,
> > though I update the obvious ones.
> > 
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Acked-by: David S. Miller <davem@davemloft.net>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> Acked-by: Asias He <asias@redhat.com>

Acked-by: Dmitry Torokhov <dtor@vmware.com>

Thanks,
Dmitry

Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec

[Michael S. Tsirkin]

On Fri, May 17, 2013 at 09:12:39AM +0930, Rusty Russell wrote:
> Joe Perches <joe@perches.com> writes:
> > On Thu, 2013-05-16 at 13:04 +0930, Rusty Russell wrote:
> >> Asias He <asias@redhat.com> writes:
> >> > On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote:
> > []
> >> > Other users are using memcpy_fromiovec and friends outside net. It seems
> >> > a good idea to put it in a util library. e.g.  crypto/algif_skcipher.c
> >> > which also depends on NET for it.
> >
> > []
> >> Subject: Hoist memcpy_fromiovec into lib/
> >
> > You'll need the "friends" memcpy_toiovec too.
> >
> > $ git grep -E \bmemcpy\w+iovec\w*"
> > crypto/algif_hash.c:    err = memcpy_toiovec(msg->msg_iov, ctx->result, len);
> > crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg)) +
> > crypto/algif_skcipher.c:                        err = memcpy_fromiovec(page_address(sg_page(sg + i)),
> > drivers/dma/iovlock.c:#include <net/tcp.h> /* for memcpy_toiovec */
> > drivers/dma/iovlock.c:          return memcpy_toiovec(iov, kdata, len);
> > drivers/dma/iovlock.c:          err = memcpy_toiovec(iov, vaddr + offset, len);
> > drivers/isdn/mISDN/socket.c:    if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
> > drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_fromiovec((u8 *)va + page_o
> > drivers/misc/vmw_vmci/vmci_queue_pair.c:                        err = memcpy_toiovec(iov, (u8 *)va + pag
> 
> Fascinating.  These all indirectly depend on NET, so there's no problem
> at the moment.  But it is a bit weird...
> 
> crypto/algif_hash.c: depends on CRYPTO_USER_API_HASH -> NET
> crypto/algif_skcipher.c: depends on CRYPTO_USER_API_SKCIPHER -> NET
> drivers/dma/iovlock.c: depends on NET_DMA -> NET
> drivers/isdn/mISDN/socket.c: depends on MISDN -> ISDN -> NET
> drivers/misc/vmw_vmci/vmci_queue_pair.c: depends on VMCI -> NET
> 
> Patch welcome.
> 
> Meanwhile, to avoid more bikeshedding I've put the patch I posted with
> all acks in my fixes branch.  One cycle through linux-next, then
> straight to Linus.

Not in 3.10-rc2 yes - still plan to merge for 3.10?

> Subject: Hoist memcpy_fromiovec into lib/
> 
> ERROR: "memcpy_fromiovec" [drivers/vhost/vhost_scsi.ko] undefined!
> 
> That function is only present with CONFIG_NET.  Turns out that
> crypto/algif_skcipher.c also uses that outside net, but it actually
> needs sockets anyway.
> 
> socket.h already include uio.h, so no callers need updating.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Acked-by: David S. Miller <davem@davemloft.net>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 428c37a..7266775 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -305,7 +305,6 @@ struct ucred {
>  
>  extern void cred_to_ucred(struct pid *pid, const struct cred *cred, struct ucred *ucred);
>  
> -extern int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  extern int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
>  			       int offset, int len);
>  extern int csum_partial_copy_fromiovecend(unsigned char *kdata, 
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 629aaf5..21628d3 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -35,4 +35,6 @@ static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
>  }
>  
>  unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len);
>  #endif
> diff --git a/lib/Makefile b/lib/Makefile
> index e9c52e1..2377211 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -9,7 +9,7 @@ endif
>  
>  lib-y := ctype.o string.o vsprintf.o cmdline.o \
>  	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
> -	 idr.o int_sqrt.o extable.o \
> +	 idr.o int_sqrt.o extable.o iovec.o \
>  	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
>  	 proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
>  	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> diff --git a/lib/iovec.c b/lib/iovec.c
> new file mode 100644
> index 0000000..632c5ea
> --- /dev/null
> +++ b/lib/iovec.c
> @@ -0,0 +1,29 @@
> +#include <linux/uaccess.h>
> +#include <linux/export.h>
> +#include <linux/uio.h>
> +
> +/*
> + *	Copy iovec to kernel. Returns -EFAULT on error.
> + *
> + *	Note: this modifies the original iovec.
> + */
> +
> +int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> +{
> +	while (len > 0) {
> +		if (iov->iov_len) {
> +			int copy = min_t(unsigned int, len, iov->iov_len);
> +			if (copy_from_user(kdata, iov->iov_base, copy))
> +				return -EFAULT;
> +			len -= copy;
> +			kdata += copy;
> +			iov->iov_base += copy;
> +			iov->iov_len -= copy;
> +		}
> +		iov++;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(memcpy_fromiovec);
> +
> diff --git a/net/core/iovec.c b/net/core/iovec.c
> index 7e7aeb0..d81257f 100644
> --- a/net/core/iovec.c
> +++ b/net/core/iovec.c
> @@ -125,31 +125,6 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
>  EXPORT_SYMBOL(memcpy_toiovecend);
>  
>  /*
> - *	Copy iovec to kernel. Returns -EFAULT on error.
> - *
> - *	Note: this modifies the original iovec.
> - */
> -
> -int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
> -{
> -	while (len > 0) {
> -		if (iov->iov_len) {
> -			int copy = min_t(unsigned int, len, iov->iov_len);
> -			if (copy_from_user(kdata, iov->iov_base, copy))
> -				return -EFAULT;
> -			len -= copy;
> -			kdata += copy;
> -			iov->iov_base += copy;
> -			iov->iov_len -= copy;
> -		}
> -		iov++;
> -	}
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcpy_fromiovec);
> -
> -/*
>   *	Copy iovec from kernel. Returns -EFAULT on error.
>   */
>