From patchwork Thu May 16 03:34:58 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rusty Russell X-Patchwork-Id: 2575031 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id AA6D0DFB7B for ; Thu, 16 May 2013 03:36:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754693Ab3EPDf4 (ORCPT ); Wed, 15 May 2013 23:35:56 -0400 Received: from ozlabs.org ([203.10.76.45]:40060 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753781Ab3EPDfy (ORCPT ); Wed, 15 May 2013 23:35:54 -0400 Received: by ozlabs.org (Postfix, from userid 1011) id 5936F2C00A4; Thu, 16 May 2013 13:35:53 +1000 (EST) From: Rusty Russell To: Asias He Cc: "Michael S. Tsirkin" , Nicholas Bellinger , kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, target-devel@vger.kernel.org, Stephen Rothwell , Randy Dunlap , linux-next@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH] vhost-scsi: Depend on NET for memcpy_fromiovec In-Reply-To: <20130516020848.GB23441@hj.localdomain> References: <20130515095558.918f2b29ba318a477eb5dde2@canb.auug.org.au> <1368579583-13097-1-git-send-email-asias@redhat.com> <8761yk254u.fsf@rustcorp.com.au> <20130516020848.GB23441@hj.localdomain> User-Agent: Notmuch/0.15.2+81~gd2c8818 (http://notmuchmail.org) Emacs/23.4.1 (i686-pc-linux-gnu) Date: Thu, 16 May 2013 13:04:58 +0930 Message-ID: <87d2sra97h.fsf@rustcorp.com.au> MIME-Version: 1.0 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Asias He writes: > On Wed, May 15, 2013 at 02:47:53PM +0930, Rusty Russell wrote: >> Asias He 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 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 Signed-off-by: Rusty Russell Acked-by: Michael S. Tsirkin --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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 +#include +#include + +/* + * 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. */