* Re: [PATCH] xen-netback: corretly check failed allocation
2015-10-15 16:26 [PATCH] xen-netback: corretly check failed allocation Insu Yun
@ 2015-10-15 16:44 ` Ian Campbell
2015-10-15 16:44 ` Ian Campbell
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Ian Campbell @ 2015-10-15 16:44 UTC (permalink / raw)
To: Insu Yun, wei.liu2, xen-devel, netdev, linux-kernel
Cc: yeongjin.jang, taesoo, insu
On Thu, 2015-10-15 at 12:26 -0400, Insu Yun wrote:
> Since vzalloc can be failed in memory pressure,
> return value should be checked and return ENOMEM.
>
> Signed-off-by: Insu Yun <wuninsu@gmail.com>
> ---
> drivers/net/xen-netback/xenbus.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen
> -netback/xenbus.c
> index 929a6e7..e288246 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -788,6 +788,11 @@ static void connect(struct backend_info *be)
> /* Use the number of queues requested by the frontend */
> be->vif->queues = vzalloc(requested_num_queues *
> sizeof(struct xenvif_queue));
> + if (!be->vif->queues) {
> + xenbus_dev_fatal(dev, -ENOMEM, "allocating queues");
> + return;
> + }
Please fix the coding style, perhaps using checkpatch.pl or by observing
the surrounding code.
Ian.
> +
> be->vif->num_queues = requested_num_queues;
> be->vif->stalled_queues = requested_num_queues;
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] xen-netback: corretly check failed allocation
2015-10-15 16:26 [PATCH] xen-netback: corretly check failed allocation Insu Yun
2015-10-15 16:44 ` Ian Campbell
@ 2015-10-15 16:44 ` Ian Campbell
2015-10-15 17:13 ` Wei Liu
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Ian Campbell @ 2015-10-15 16:44 UTC (permalink / raw)
To: Insu Yun, wei.liu2, xen-devel, netdev, linux-kernel
Cc: taesoo, yeongjin.jang, insu
On Thu, 2015-10-15 at 12:26 -0400, Insu Yun wrote:
> Since vzalloc can be failed in memory pressure,
> return value should be checked and return ENOMEM.
>
> Signed-off-by: Insu Yun <wuninsu@gmail.com>
> ---
> drivers/net/xen-netback/xenbus.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen
> -netback/xenbus.c
> index 929a6e7..e288246 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -788,6 +788,11 @@ static void connect(struct backend_info *be)
> /* Use the number of queues requested by the frontend */
> be->vif->queues = vzalloc(requested_num_queues *
> sizeof(struct xenvif_queue));
> + if (!be->vif->queues) {
> + xenbus_dev_fatal(dev, -ENOMEM, "allocating queues");
> + return;
> + }
Please fix the coding style, perhaps using checkpatch.pl or by observing
the surrounding code.
Ian.
> +
> be->vif->num_queues = requested_num_queues;
> be->vif->stalled_queues = requested_num_queues;
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] xen-netback: corretly check failed allocation
2015-10-15 16:26 [PATCH] xen-netback: corretly check failed allocation Insu Yun
2015-10-15 16:44 ` Ian Campbell
2015-10-15 16:44 ` Ian Campbell
@ 2015-10-15 17:13 ` Wei Liu
2015-10-15 17:13 ` Wei Liu
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Wei Liu @ 2015-10-15 17:13 UTC (permalink / raw)
To: Insu Yun
Cc: yeongjin.jang, taesoo, wei.liu2, ian.campbell, netdev, insu,
linux-kernel, xen-devel
On Thu, Oct 15, 2015 at 12:26:16PM -0400, Insu Yun wrote:
> Since vzalloc can be failed in memory pressure,
> return value should be checked and return ENOMEM.
This function doesn't return ENOMEM, instead it writes to xenstore to
indicate error. The commit log needs to be updated.
>
> Signed-off-by: Insu Yun <wuninsu@gmail.com>
> ---
> drivers/net/xen-netback/xenbus.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 929a6e7..e288246 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -788,6 +788,11 @@ static void connect(struct backend_info *be)
> /* Use the number of queues requested by the frontend */
> be->vif->queues = vzalloc(requested_num_queues *
> sizeof(struct xenvif_queue));
> + if (!be->vif->queues) {
> + xenbus_dev_fatal(dev, -ENOMEM, "allocating queues");
> + return;
> + }
> +
The indentation is wrong. Please configure your email client properly.
And please use "goto err" for error handling -- yes, I understand there
is existing code that returns directly but IMHO that should be fixed
too.
We.
> be->vif->num_queues = requested_num_queues;
> be->vif->stalled_queues = requested_num_queues;
>
> --
> 1.9.1
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] xen-netback: corretly check failed allocation
2015-10-15 16:26 [PATCH] xen-netback: corretly check failed allocation Insu Yun
` (2 preceding siblings ...)
2015-10-15 17:13 ` Wei Liu
@ 2015-10-15 17:13 ` Wei Liu
2015-10-15 17:27 ` Insu Yun
2015-10-16 6:32 ` David Miller
2015-10-16 6:32 ` David Miller
5 siblings, 1 reply; 9+ messages in thread
From: Wei Liu @ 2015-10-15 17:13 UTC (permalink / raw)
To: Insu Yun
Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel, taesoo,
yeongjin.jang, insu
On Thu, Oct 15, 2015 at 12:26:16PM -0400, Insu Yun wrote:
> Since vzalloc can be failed in memory pressure,
> return value should be checked and return ENOMEM.
This function doesn't return ENOMEM, instead it writes to xenstore to
indicate error. The commit log needs to be updated.
>
> Signed-off-by: Insu Yun <wuninsu@gmail.com>
> ---
> drivers/net/xen-netback/xenbus.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 929a6e7..e288246 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -788,6 +788,11 @@ static void connect(struct backend_info *be)
> /* Use the number of queues requested by the frontend */
> be->vif->queues = vzalloc(requested_num_queues *
> sizeof(struct xenvif_queue));
> + if (!be->vif->queues) {
> + xenbus_dev_fatal(dev, -ENOMEM, "allocating queues");
> + return;
> + }
> +
The indentation is wrong. Please configure your email client properly.
And please use "goto err" for error handling -- yes, I understand there
is existing code that returns directly but IMHO that should be fixed
too.
We.
> be->vif->num_queues = requested_num_queues;
> be->vif->stalled_queues = requested_num_queues;
>
> --
> 1.9.1
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] xen-netback: corretly check failed allocation
2015-10-15 17:13 ` Wei Liu
@ 2015-10-15 17:27 ` Insu Yun
0 siblings, 0 replies; 9+ messages in thread
From: Insu Yun @ 2015-10-15 17:27 UTC (permalink / raw)
To: Wei Liu
Cc: Yeongjin Jang, Taesoo Kim, Ian Campbell, netdev, Yun, Insu,
linux-kernel, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1659 bytes --]
Sorry for wrong format, I forgot to change my configuration.
On Thu, Oct 15, 2015 at 1:13 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Thu, Oct 15, 2015 at 12:26:16PM -0400, Insu Yun wrote:
> > Since vzalloc can be failed in memory pressure,
> > return value should be checked and return ENOMEM.
>
> This function doesn't return ENOMEM, instead it writes to xenstore to
> indicate error. The commit log needs to be updated.
>
>
Good. I will update it.
> >
> > Signed-off-by: Insu Yun <wuninsu@gmail.com>
> > ---
> > drivers/net/xen-netback/xenbus.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/xen-netback/xenbus.c
> b/drivers/net/xen-netback/xenbus.c
> > index 929a6e7..e288246 100644
> > --- a/drivers/net/xen-netback/xenbus.c
> > +++ b/drivers/net/xen-netback/xenbus.c
> > @@ -788,6 +788,11 @@ static void connect(struct backend_info *be)
> > /* Use the number of queues requested by the frontend */
> > be->vif->queues = vzalloc(requested_num_queues *
> > sizeof(struct xenvif_queue));
> > + if (!be->vif->queues) {
> > + xenbus_dev_fatal(dev, -ENOMEM, "allocating queues");
> > + return;
> > + }
> > +
>
> The indentation is wrong. Please configure your email client properly.
>
> And please use "goto err" for error handling -- yes, I understand there
> is existing code that returns directly but IMHO that should be fixed
> too.
>
>
Ok. Actually I made a code referring the existing code.
> We.
>
> > be->vif->num_queues = requested_num_queues;
> > be->vif->stalled_queues = requested_num_queues;
> >
> > --
> > 1.9.1
>
--
Regards
Insu Yun
[-- Attachment #1.2: Type: text/html, Size: 2802 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xen-netback: corretly check failed allocation
2015-10-15 16:26 [PATCH] xen-netback: corretly check failed allocation Insu Yun
` (3 preceding siblings ...)
2015-10-15 17:13 ` Wei Liu
@ 2015-10-16 6:32 ` David Miller
2015-10-16 6:32 ` David Miller
5 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2015-10-16 6:32 UTC (permalink / raw)
To: wuninsu
Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel, taesoo,
yeongjin.jang, insu
From: Insu Yun <wuninsu@gmail.com>
Date: Thu, 15 Oct 2015 12:26:16 -0400
> Since vzalloc can be failed in memory pressure,
> return value should be checked and return ENOMEM.
>
> Signed-off-by: Insu Yun <wuninsu@gmail.com>
> ---
> drivers/net/xen-netback/xenbus.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 929a6e7..e288246 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -788,6 +788,11 @@ static void connect(struct backend_info *be)
> /* Use the number of queues requested by the frontend */
> be->vif->queues = vzalloc(requested_num_queues *
> sizeof(struct xenvif_queue));
> + if (!be->vif->queues) {
> + xenbus_dev_fatal(dev, -ENOMEM, "allocating queues");
> + return;
> + }
This is definitely not indented correctly.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] xen-netback: corretly check failed allocation
2015-10-15 16:26 [PATCH] xen-netback: corretly check failed allocation Insu Yun
` (4 preceding siblings ...)
2015-10-16 6:32 ` David Miller
@ 2015-10-16 6:32 ` David Miller
5 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2015-10-16 6:32 UTC (permalink / raw)
To: wuninsu
Cc: yeongjin.jang, taesoo, wei.liu2, ian.campbell, netdev, insu,
linux-kernel, xen-devel
From: Insu Yun <wuninsu@gmail.com>
Date: Thu, 15 Oct 2015 12:26:16 -0400
> Since vzalloc can be failed in memory pressure,
> return value should be checked and return ENOMEM.
>
> Signed-off-by: Insu Yun <wuninsu@gmail.com>
> ---
> drivers/net/xen-netback/xenbus.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 929a6e7..e288246 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -788,6 +788,11 @@ static void connect(struct backend_info *be)
> /* Use the number of queues requested by the frontend */
> be->vif->queues = vzalloc(requested_num_queues *
> sizeof(struct xenvif_queue));
> + if (!be->vif->queues) {
> + xenbus_dev_fatal(dev, -ENOMEM, "allocating queues");
> + return;
> + }
This is definitely not indented correctly.
^ permalink raw reply [flat|nested] 9+ messages in thread