* [PATCH 0/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
@ 2013-01-21 8:05 Asias He
2013-01-21 8:05 ` [PATCH 1/3] tcm_vhost: Introduce iov_num_pages Asias He
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Asias He @ 2013-01-21 8:05 UTC (permalink / raw)
To: Nicholas Bellinger
Cc: Michael S. Tsirkin, Rusty Russell, kvm, virtualization, target-devel
Asias He (3):
tcm_vhost: Introduce iov_num_pages
tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
tcm_vhost: Use iov_num_pages to calculate sgl_count
drivers/vhost/tcm_vhost.c | 47 +++++++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 18 deletions(-)
--
1.8.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] tcm_vhost: Introduce iov_num_pages
2013-01-21 8:05 [PATCH 0/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
@ 2013-01-21 8:05 ` Asias He
2013-01-21 8:05 ` [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Asias He @ 2013-01-21 8:05 UTC (permalink / raw)
To: Nicholas Bellinger; +Cc: target-devel, virtualization, kvm, Michael S. Tsirkin
Add a helper to calculate the number of pages needed for a iov entry.
Signed-off-by: Asias He <asias@redhat.com>
---
drivers/vhost/tcm_vhost.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 3720604..ca35c16 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -77,6 +77,12 @@ static struct workqueue_struct *tcm_vhost_workqueue;
static DEFINE_MUTEX(tcm_vhost_mutex);
static LIST_HEAD(tcm_vhost_list);
+static inline int iov_num_pages(struct iovec *iov)
+{
+ return (PAGE_ALIGN((unsigned long)iov->iov_base + iov->iov_len) -
+ ((unsigned long)iov->iov_base & PAGE_MASK)) >> PAGE_SHIFT;
+}
+
static int tcm_vhost_check_true(struct se_portal_group *se_tpg)
{
return 1;
--
1.8.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
2013-01-21 8:05 [PATCH 0/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
2013-01-21 8:05 ` [PATCH 1/3] tcm_vhost: Introduce iov_num_pages Asias He
@ 2013-01-21 8:05 ` Asias He
2013-01-21 18:57 ` Marcelo Tosatti
2013-01-21 18:57 ` Marcelo Tosatti
2013-01-21 8:05 ` Asias He
2013-01-21 8:05 ` [PATCH 3/3] tcm_vhost: Use iov_num_pages to calculate sgl_count Asias He
3 siblings, 2 replies; 9+ messages in thread
From: Asias He @ 2013-01-21 8:05 UTC (permalink / raw)
To: Nicholas Bellinger
Cc: Michael S. Tsirkin, Rusty Russell, kvm, virtualization, target-devel
We can get all the pages in one time instead of calling
gup N times.
Signed-off-by: Asias He <asias@redhat.com>
---
drivers/vhost/tcm_vhost.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index ca35c16..59be442 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -430,37 +430,45 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
* Returns the number of scatterlist entries used or -errno on error.
*/
static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
- unsigned int sgl_count, void __user *ptr, size_t len, int write)
+ unsigned int sgl_count, struct iovec *iov, int write)
{
struct scatterlist *sg = sgl;
unsigned int npages = 0;
+ void __user *ptr = iov->iov_base;
+ size_t len = iov->iov_len;
int ret;
+ unsigned int pages_nr, offset, nbytes;
+ struct page **pages;
+
+ pages_nr = iov_num_pages(iov);
+ pages = kmalloc(pages_nr * sizeof(struct page *), GFP_ATOMIC);
+ if (!pages)
+ return -ENOMEM;
+
+ ret = get_user_pages_fast((unsigned long)ptr, pages_nr, write, pages);
+ if (ret != pages_nr)
+ goto err;
while (len > 0) {
- struct page *page;
- unsigned int offset = (uintptr_t)ptr & ~PAGE_MASK;
- unsigned int nbytes = min_t(unsigned int,
- PAGE_SIZE - offset, len);
+ offset = (uintptr_t)ptr & ~PAGE_MASK;
+ nbytes = min_t(unsigned int, PAGE_SIZE - offset, len);
if (npages == sgl_count) {
ret = -ENOBUFS;
goto err;
}
- ret = get_user_pages_fast((unsigned long)ptr, 1, write, &page);
- BUG_ON(ret == 0); /* we should either get our page or fail */
- if (ret < 0)
- goto err;
-
- sg_set_page(sg, page, nbytes, offset);
+ sg_set_page(sg, pages[npages], nbytes, offset);
ptr += nbytes;
len -= nbytes;
sg++;
npages++;
}
+ kfree(pages);
return npages;
err:
+ kfree(pages);
/* Put pages that we hold */
for (sg = sgl; sg != &sgl[npages]; sg++)
put_page(sg_page(sg));
@@ -498,8 +506,7 @@ static int vhost_scsi_map_iov_to_sgl(struct tcm_vhost_cmd *tv_cmd,
pr_debug("Mapping %u iovecs for %u pages\n", niov, sgl_count);
for (i = 0; i < niov; i++) {
- ret = vhost_scsi_map_to_sgl(sg, sgl_count, iov[i].iov_base,
- iov[i].iov_len, write);
+ ret = vhost_scsi_map_to_sgl(sg, sgl_count, &iov[i], write);
if (ret < 0) {
for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
put_page(sg_page(&tv_cmd->tvc_sgl[i]));
--
1.8.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
2013-01-21 8:05 [PATCH 0/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
2013-01-21 8:05 ` [PATCH 1/3] tcm_vhost: Introduce iov_num_pages Asias He
2013-01-21 8:05 ` [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
@ 2013-01-21 8:05 ` Asias He
2013-01-21 8:05 ` [PATCH 3/3] tcm_vhost: Use iov_num_pages to calculate sgl_count Asias He
3 siblings, 0 replies; 9+ messages in thread
From: Asias He @ 2013-01-21 8:05 UTC (permalink / raw)
To: Nicholas Bellinger; +Cc: target-devel, virtualization, kvm, Michael S. Tsirkin
We can get all the pages in one time instead of calling
gup N times.
Signed-off-by: Asias He <asias@redhat.com>
---
drivers/vhost/tcm_vhost.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index ca35c16..59be442 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -430,37 +430,45 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
* Returns the number of scatterlist entries used or -errno on error.
*/
static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
- unsigned int sgl_count, void __user *ptr, size_t len, int write)
+ unsigned int sgl_count, struct iovec *iov, int write)
{
struct scatterlist *sg = sgl;
unsigned int npages = 0;
+ void __user *ptr = iov->iov_base;
+ size_t len = iov->iov_len;
int ret;
+ unsigned int pages_nr, offset, nbytes;
+ struct page **pages;
+
+ pages_nr = iov_num_pages(iov);
+ pages = kmalloc(pages_nr * sizeof(struct page *), GFP_ATOMIC);
+ if (!pages)
+ return -ENOMEM;
+
+ ret = get_user_pages_fast((unsigned long)ptr, pages_nr, write, pages);
+ if (ret != pages_nr)
+ goto err;
while (len > 0) {
- struct page *page;
- unsigned int offset = (uintptr_t)ptr & ~PAGE_MASK;
- unsigned int nbytes = min_t(unsigned int,
- PAGE_SIZE - offset, len);
+ offset = (uintptr_t)ptr & ~PAGE_MASK;
+ nbytes = min_t(unsigned int, PAGE_SIZE - offset, len);
if (npages == sgl_count) {
ret = -ENOBUFS;
goto err;
}
- ret = get_user_pages_fast((unsigned long)ptr, 1, write, &page);
- BUG_ON(ret == 0); /* we should either get our page or fail */
- if (ret < 0)
- goto err;
-
- sg_set_page(sg, page, nbytes, offset);
+ sg_set_page(sg, pages[npages], nbytes, offset);
ptr += nbytes;
len -= nbytes;
sg++;
npages++;
}
+ kfree(pages);
return npages;
err:
+ kfree(pages);
/* Put pages that we hold */
for (sg = sgl; sg != &sgl[npages]; sg++)
put_page(sg_page(sg));
@@ -498,8 +506,7 @@ static int vhost_scsi_map_iov_to_sgl(struct tcm_vhost_cmd *tv_cmd,
pr_debug("Mapping %u iovecs for %u pages\n", niov, sgl_count);
for (i = 0; i < niov; i++) {
- ret = vhost_scsi_map_to_sgl(sg, sgl_count, iov[i].iov_base,
- iov[i].iov_len, write);
+ ret = vhost_scsi_map_to_sgl(sg, sgl_count, &iov[i], write);
if (ret < 0) {
for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
put_page(sg_page(&tv_cmd->tvc_sgl[i]));
--
1.8.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] tcm_vhost: Use iov_num_pages to calculate sgl_count
2013-01-21 8:05 [PATCH 0/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
` (2 preceding siblings ...)
2013-01-21 8:05 ` Asias He
@ 2013-01-21 8:05 ` Asias He
3 siblings, 0 replies; 9+ messages in thread
From: Asias He @ 2013-01-21 8:05 UTC (permalink / raw)
To: Nicholas Bellinger; +Cc: target-devel, virtualization, kvm, Michael S. Tsirkin
Signed-off-by: Asias He <asias@redhat.com>
---
drivers/vhost/tcm_vhost.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 59be442..5b77469 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -487,11 +487,9 @@ static int vhost_scsi_map_iov_to_sgl(struct tcm_vhost_cmd *tv_cmd,
* Find out how long sglist needs to be
*/
sgl_count = 0;
- for (i = 0; i < niov; i++) {
- sgl_count += (((uintptr_t)iov[i].iov_base + iov[i].iov_len +
- PAGE_SIZE - 1) >> PAGE_SHIFT) -
- ((uintptr_t)iov[i].iov_base >> PAGE_SHIFT);
- }
+ for (i = 0; i < niov; i++)
+ sgl_count += iov_num_pages(&iov[i]);
+
/* TODO overflow checking */
sg = kmalloc(sizeof(tv_cmd->tvc_sgl[0]) * sgl_count, GFP_ATOMIC);
--
1.8.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
2013-01-21 8:05 ` [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
@ 2013-01-21 18:57 ` Marcelo Tosatti
2013-01-22 1:33 ` Asias He
2013-01-21 18:57 ` Marcelo Tosatti
1 sibling, 1 reply; 9+ messages in thread
From: Marcelo Tosatti @ 2013-01-21 18:57 UTC (permalink / raw)
To: Asias He
Cc: Nicholas Bellinger, Michael S. Tsirkin, Rusty Russell, kvm,
virtualization, target-devel
On Mon, Jan 21, 2013 at 04:05:27PM +0800, Asias He wrote:
> We can get all the pages in one time instead of calling
> gup N times.
>
> Signed-off-by: Asias He <asias@redhat.com>
> ---
> drivers/vhost/tcm_vhost.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> index ca35c16..59be442 100644
> --- a/drivers/vhost/tcm_vhost.c
> +++ b/drivers/vhost/tcm_vhost.c
> @@ -430,37 +430,45 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
> * Returns the number of scatterlist entries used or -errno on error.
> */
> static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
> - unsigned int sgl_count, void __user *ptr, size_t len, int write)
> + unsigned int sgl_count, struct iovec *iov, int write)
> {
> struct scatterlist *sg = sgl;
> unsigned int npages = 0;
> + void __user *ptr = iov->iov_base;
> + size_t len = iov->iov_len;
> int ret;
> + unsigned int pages_nr, offset, nbytes;
> + struct page **pages;
> +
> + pages_nr = iov_num_pages(iov);
> + pages = kmalloc(pages_nr * sizeof(struct page *), GFP_ATOMIC);
> + if (!pages)
> + return -ENOMEM;
> +
> + ret = get_user_pages_fast((unsigned long)ptr, pages_nr, write, pages);
> + if (ret != pages_nr)
> + goto err;
1. Why GFP_ATOMIC? get_user_pages_fast can sleep, so this path must not
be atomic (if it is, should use __get_user_pages_fast).
GFP_ATOMIC should be avoided.
2. Should drop reference to pages whose refcount has been increased,
if ret > 0 && ret != pages_nr (see last phrase of get_user_pages_fast
commentary).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
2013-01-21 8:05 ` [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
2013-01-21 18:57 ` Marcelo Tosatti
@ 2013-01-21 18:57 ` Marcelo Tosatti
1 sibling, 0 replies; 9+ messages in thread
From: Marcelo Tosatti @ 2013-01-21 18:57 UTC (permalink / raw)
To: Asias He; +Cc: kvm, Michael S. Tsirkin, virtualization, target-devel
On Mon, Jan 21, 2013 at 04:05:27PM +0800, Asias He wrote:
> We can get all the pages in one time instead of calling
> gup N times.
>
> Signed-off-by: Asias He <asias@redhat.com>
> ---
> drivers/vhost/tcm_vhost.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> index ca35c16..59be442 100644
> --- a/drivers/vhost/tcm_vhost.c
> +++ b/drivers/vhost/tcm_vhost.c
> @@ -430,37 +430,45 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
> * Returns the number of scatterlist entries used or -errno on error.
> */
> static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
> - unsigned int sgl_count, void __user *ptr, size_t len, int write)
> + unsigned int sgl_count, struct iovec *iov, int write)
> {
> struct scatterlist *sg = sgl;
> unsigned int npages = 0;
> + void __user *ptr = iov->iov_base;
> + size_t len = iov->iov_len;
> int ret;
> + unsigned int pages_nr, offset, nbytes;
> + struct page **pages;
> +
> + pages_nr = iov_num_pages(iov);
> + pages = kmalloc(pages_nr * sizeof(struct page *), GFP_ATOMIC);
> + if (!pages)
> + return -ENOMEM;
> +
> + ret = get_user_pages_fast((unsigned long)ptr, pages_nr, write, pages);
> + if (ret != pages_nr)
> + goto err;
1. Why GFP_ATOMIC? get_user_pages_fast can sleep, so this path must not
be atomic (if it is, should use __get_user_pages_fast).
GFP_ATOMIC should be avoided.
2. Should drop reference to pages whose refcount has been increased,
if ret > 0 && ret != pages_nr (see last phrase of get_user_pages_fast
commentary).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
2013-01-21 18:57 ` Marcelo Tosatti
@ 2013-01-22 1:33 ` Asias He
0 siblings, 0 replies; 9+ messages in thread
From: Asias He @ 2013-01-22 1:33 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, virtualization, target-devel
On 01/22/2013 02:57 AM, Marcelo Tosatti wrote:
> On Mon, Jan 21, 2013 at 04:05:27PM +0800, Asias He wrote:
>> We can get all the pages in one time instead of calling
>> gup N times.
>>
>> Signed-off-by: Asias He <asias@redhat.com>
>> ---
>> drivers/vhost/tcm_vhost.c | 33 ++++++++++++++++++++-------------
>> 1 file changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
>> index ca35c16..59be442 100644
>> --- a/drivers/vhost/tcm_vhost.c
>> +++ b/drivers/vhost/tcm_vhost.c
>> @@ -430,37 +430,45 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
>> * Returns the number of scatterlist entries used or -errno on error.
>> */
>> static int vhost_scsi_map_to_sgl(struct scatterlist *sgl,
>> - unsigned int sgl_count, void __user *ptr, size_t len, int write)
>> + unsigned int sgl_count, struct iovec *iov, int write)
>> {
>> struct scatterlist *sg = sgl;
>> unsigned int npages = 0;
>> + void __user *ptr = iov->iov_base;
>> + size_t len = iov->iov_len;
>> int ret;
>> + unsigned int pages_nr, offset, nbytes;
>> + struct page **pages;
>> +
>> + pages_nr = iov_num_pages(iov);
>> + pages = kmalloc(pages_nr * sizeof(struct page *), GFP_ATOMIC);
>> + if (!pages)
>> + return -ENOMEM;
>> +
>> + ret = get_user_pages_fast((unsigned long)ptr, pages_nr, write, pages);
>> + if (ret != pages_nr)
>> + goto err;
>
> 1. Why GFP_ATOMIC? get_user_pages_fast can sleep, so this path must not
> be atomic (if it is, should use __get_user_pages_fast).
>
> GFP_ATOMIC should be avoided.
>
> 2. Should drop reference to pages whose refcount has been increased,
> if ret > 0 && ret != pages_nr (see last phrase of get_user_pages_fast
> commentary).
Thanks Marcelo. V2 is on the way.
--
Asias
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
@ 2013-01-21 8:05 Asias He
0 siblings, 0 replies; 9+ messages in thread
From: Asias He @ 2013-01-21 8:05 UTC (permalink / raw)
To: Nicholas Bellinger; +Cc: target-devel, virtualization, kvm, Michael S. Tsirkin
Asias He (3):
tcm_vhost: Introduce iov_num_pages
tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl
tcm_vhost: Use iov_num_pages to calculate sgl_count
drivers/vhost/tcm_vhost.c | 47 +++++++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 18 deletions(-)
--
1.8.1
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-01-22 1:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-21 8:05 [PATCH 0/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
2013-01-21 8:05 ` [PATCH 1/3] tcm_vhost: Introduce iov_num_pages Asias He
2013-01-21 8:05 ` [PATCH 2/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
2013-01-21 18:57 ` Marcelo Tosatti
2013-01-22 1:33 ` Asias He
2013-01-21 18:57 ` Marcelo Tosatti
2013-01-21 8:05 ` Asias He
2013-01-21 8:05 ` [PATCH 3/3] tcm_vhost: Use iov_num_pages to calculate sgl_count Asias He
2013-01-21 8:05 [PATCH 0/3] tcm_vhost: Optimize gup in vhost_scsi_map_to_sgl Asias He
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.