thr3ads.net - Linux Virtualization - [PATCH] vhost/scsi: use vmalloc for order-10 allocation [Sep 2013]

If this information is useful, please help other people find it:
Share via:

Michael S. Tsirkin

2013-Sep-17 07:21 UTC

[PATCH] vhost/scsi: use vmalloc for order-10 allocation

As vhost scsi device struct is large, if the device is
created on a busy system, kzalloc() might fail, so this patch does a
fallback to vzalloc().

As vmalloc() adds overhead on data-path, add __GFP_REPEAT
to kzalloc() flags to do this fallback only when really needed.

Reported-by: Dan Aloni <alonid at stratoscale.com>
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---

I put this on my vhost fixes branch, intend to merge for 3.12.
Dan, could you please confirm this works for you?

 drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 4b79a1f..2c30bb0 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct vhost_scsi
*vs, u64 features)
 	return 0;
 }
 
+static void vhost_scsi_free(struct vhost_scsi *vs)
+{
+        if (is_vmalloc_addr(vs))
+                vfree(vs);
+        else
+                kfree(vs);
+}
+
 static int vhost_scsi_open(struct inode *inode, struct file *f)
 {
 	struct vhost_scsi *vs;
 	struct vhost_virtqueue **vqs;
-	int r, i;
+	int r = -ENOMEM, i;
 
-	vs = kzalloc(sizeof(*vs), GFP_KERNEL);
-	if (!vs)
-		return -ENOMEM;
+        vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!vs) {
+		vs = vzalloc(sizeof(*vs));
+		if (!vs)
+			goto err_vs;
+	}
 
 	vqs = kmalloc(VHOST_SCSI_MAX_VQ * sizeof(*vqs), GFP_KERNEL);
-	if (!vqs) {
-		kfree(vs);
-		return -ENOMEM;
-	}
+	if (!vqs)
+		goto err_vqs;
 
 	vhost_work_init(&vs->vs_completion_work, vhost_scsi_complete_cmd_work);
 	vhost_work_init(&vs->vs_event_work, tcm_vhost_evt_work);
@@ -1407,14 +1416,18 @@ static int vhost_scsi_open(struct inode *inode, struct
file *f)
 
 	tcm_vhost_init_inflight(vs, NULL);
 
-	if (r < 0) {
-		kfree(vqs);
-		kfree(vs);
-		return r;
-	}
+	if (r < 0)
+		goto err_init;
 
 	f->private_data = vs;
 	return 0;
+
+err_init:
+	kfree(vqs);
+err_vqs:
+	vhost_scsi_free(vs);
+err_vs:
+	return r;
 }
 
 static int vhost_scsi_release(struct inode *inode, struct file *f)
@@ -1431,7 +1444,7 @@ static int vhost_scsi_release(struct inode *inode, struct
file *f)
 	/* Jobs can re-queue themselves in evt kick handler. Do extra flush. */
 	vhost_scsi_flush(vs);
 	kfree(vs->dev.vqs);
-	kfree(vs);
+	vhost_scsi_free(vs);
 	return 0;
 }
 
-- 
MST

Asias He

2013-Sep-17 08:55 UTC

head link

[PATCH] vhost/scsi: use vmalloc for order-10 allocation

On Tue, Sep 17, 2013 at 10:21:07AM +0300, Michael S. Tsirkin
wrote:> As vhost scsi device struct is large, if the device is
> created on a busy system, kzalloc() might fail, so this patch does a
> fallback to vzalloc().
> 
> As vmalloc() adds overhead on data-path, add __GFP_REPEAT
> to kzalloc() flags to do this fallback only when really needed.
> 
> Reported-by: Dan Aloni <alonid at stratoscale.com>
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
Reviewed-by: Asias He <asias at redhat.com>
> ---
> 
> I put this on my vhost fixes branch, intend to merge for 3.12.
> Dan, could you please confirm this works for you?
> 
>  drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 4b79a1f..2c30bb0 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct
vhost_scsi *vs, u64 features)
>  	return 0;
>  }
>  
> +static void vhost_scsi_free(struct vhost_scsi *vs)
> +{
> +        if (is_vmalloc_addr(vs))
> +                vfree(vs);
> +        else
> +                kfree(vs);
> +}
> +
>  static int vhost_scsi_open(struct inode *inode, struct file *f)
>  {
>  	struct vhost_scsi *vs;
>  	struct vhost_virtqueue **vqs;
> -	int r, i;
> +	int r = -ENOMEM, i;
>  
> -	vs = kzalloc(sizeof(*vs), GFP_KERNEL);
> -	if (!vs)
> -		return -ENOMEM;
> +        vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN |
__GFP_REPEAT);
> +	if (!vs) {
> +		vs = vzalloc(sizeof(*vs));
> +		if (!vs)
> +			goto err_vs;
> +	}
>  
>  	vqs = kmalloc(VHOST_SCSI_MAX_VQ * sizeof(*vqs), GFP_KERNEL);
> -	if (!vqs) {
> -		kfree(vs);
> -		return -ENOMEM;
> -	}
> +	if (!vqs)
> +		goto err_vqs;
>  
>  	vhost_work_init(&vs->vs_completion_work,
vhost_scsi_complete_cmd_work);
>  	vhost_work_init(&vs->vs_event_work, tcm_vhost_evt_work);
> @@ -1407,14 +1416,18 @@ static int vhost_scsi_open(struct inode *inode,
struct file *f)
>  
>  	tcm_vhost_init_inflight(vs, NULL);
>  
> -	if (r < 0) {
> -		kfree(vqs);
> -		kfree(vs);
> -		return r;
> -	}
> +	if (r < 0)
> +		goto err_init;
>  
>  	f->private_data = vs;
>  	return 0;
> +
> +err_init:
> +	kfree(vqs);
> +err_vqs:
> +	vhost_scsi_free(vs);
> +err_vs:
> +	return r;
>  }
>  
>  static int vhost_scsi_release(struct inode *inode, struct file *f)
> @@ -1431,7 +1444,7 @@ static int vhost_scsi_release(struct inode *inode,
struct file *f)
>  	/* Jobs can re-queue themselves in evt kick handler. Do extra flush. */
>  	vhost_scsi_flush(vs);
>  	kfree(vs->dev.vqs);
> -	kfree(vs);
> +	vhost_scsi_free(vs);
>  	return 0;
>  }
>  
> -- 
> MST
-- 
Asias

Sergei Shtylyov

2013-Sep-17 18:14 UTC

head link

[PATCH] vhost/scsi: use vmalloc for order-10 allocation

Hello.

On 09/17/2013 11:21 AM, Michael S. Tsirkin wrote:
> As vhost scsi device struct is large, if the device is
> created on a busy system, kzalloc() might fail, so this patch does a
> fallback to vzalloc().
> As vmalloc() adds overhead on data-path, add __GFP_REPEAT
> to kzalloc() flags to do this fallback only when really needed.
> Reported-by: Dan Aloni <alonid at stratoscale.com>
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
> I put this on my vhost fixes branch, intend to merge for 3.12.
> Dan, could you please confirm this works for you?
>   drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
>   1 file changed, 27 insertions(+), 14 deletions(-)
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 4b79a1f..2c30bb0 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct
vhost_scsi *vs, u64 features)
>   	return 0;
>   }
>
> +static void vhost_scsi_free(struct vhost_scsi *vs)
> +{
> +        if (is_vmalloc_addr(vs))
> +                vfree(vs);
> +        else
> +                kfree(vs);
    Indent with the tabs ISO spaces, please.
> +}
> +
>   static int vhost_scsi_open(struct inode *inode, struct file *f)
>   {
>   	struct vhost_scsi *vs;
>   	struct vhost_virtqueue **vqs;
> -	int r, i;
> +	int r = -ENOMEM, i;
>
> -	vs = kzalloc(sizeof(*vs), GFP_KERNEL);
> -	if (!vs)
> -		return -ENOMEM;
> +        vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN |
__GFP_REPEAT);
    Indent here with a tab, please.
> +	if (!vs) {
> +		vs = vzalloc(sizeof(*vs));
> +		if (!vs)
> +			goto err_vs;
> +	}
WBR, Sergei

Michael S. Tsirkin

2013-Sep-17 19:51 UTC

head link

[PATCH] vhost/scsi: use vmalloc for order-10 allocation

On Tue, Sep 17, 2013 at 10:14:37PM +0400, Sergei Shtylyov
wrote:> Hello.
> 
> On 09/17/2013 11:21 AM, Michael S. Tsirkin wrote:
> 
> >As vhost scsi device struct is large, if the device is
> >created on a busy system, kzalloc() might fail, so this patch does a
> >fallback to vzalloc().
> 
> >As vmalloc() adds overhead on data-path, add __GFP_REPEAT
> >to kzalloc() flags to do this fallback only when really needed.
> 
> >Reported-by: Dan Aloni <alonid at stratoscale.com>
> >Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> >---
> 
> >I put this on my vhost fixes branch, intend to merge for 3.12.
> >Dan, could you please confirm this works for you?
> 
> >  drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
> >  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> >diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> >index 4b79a1f..2c30bb0 100644
> >--- a/drivers/vhost/scsi.c
> >+++ b/drivers/vhost/scsi.c
> >@@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct
vhost_scsi *vs, u64 features)
> >  	return 0;
> >  }
> >
> >+static void vhost_scsi_free(struct vhost_scsi *vs)
> >+{
> >+        if (is_vmalloc_addr(vs))
> >+                vfree(vs);
> >+        else
> >+                kfree(vs);
> 
>    Indent with the tabs ISO spaces, please.
> 
> >+}
> >+
> >  static int vhost_scsi_open(struct inode *inode, struct file *f)
> >  {
> >  	struct vhost_scsi *vs;
> >  	struct vhost_virtqueue **vqs;
> >-	int r, i;
> >+	int r = -ENOMEM, i;
> >
> >-	vs = kzalloc(sizeof(*vs), GFP_KERNEL);
> >-	if (!vs)
> >-		return -ENOMEM;
> >+        vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN |
__GFP_REPEAT);
> 
>    Indent here with a tab, please.
> 
> >+	if (!vs) {
> >+		vs = vzalloc(sizeof(*vs));
> >+		if (!vs)
> >+			goto err_vs;
> >+	}
> 
> WBR, Sergei
Thanks, I'll fix this up.

Maybe Matching Threads

Search for more apparently analagous threads

Linux Virtualization - Sep 2013 - [PATCH] vhost/scsi: use vmalloc for order-10 allocation

[PATCH] vhost/scsi: use vmalloc for order-10 allocation

[PATCH] vhost/scsi: use vmalloc for order-10 allocation

[PATCH] vhost/scsi: use vmalloc for order-10 allocation

[PATCH] vhost/scsi: use vmalloc for order-10 allocation

Maybe Matching Threads