* [PATCH] virtio: Return correct errno for function init_vq's failure
@ 2016-06-27 2:09 Minfei Huang
2016-07-06 9:18 ` Cornelia Huck
0 siblings, 1 reply; 4+ messages in thread
From: Minfei Huang @ 2016-06-27 2:09 UTC (permalink / raw)
To: mst; +Cc: virtualization, linux-kernel, Minfei Huang, Minfei Huang
The error number -ENOENT or 0 will be returned, if we can not allocate
more memory in function init_vq. If host can support multiple virtual
queues, and we fails to allocate necessary memory structures for vq,
kernel may crash due to incorrect returning.
To fix it, kernel will return correct value in init_vq.
Signed-off-by: Minfei Huang <mnghuan@gmail.com>
Signed-off-by: Minfei Huang <minfei.hmf@alibaba-inc.com>
---
drivers/block/virtio_blk.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 42758b5..40ecb2b 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -393,11 +393,10 @@ static int init_vq(struct virtio_blk *vblk)
if (err)
num_vqs = 1;
+ err = -ENOMEM;
vblk->vqs = kmalloc(sizeof(*vblk->vqs) * num_vqs, GFP_KERNEL);
- if (!vblk->vqs) {
- err = -ENOMEM;
+ if (!vblk->vqs)
goto out;
- }
names = kmalloc(sizeof(*names) * num_vqs, GFP_KERNEL);
if (!names)
--
2.7.4 (Apple Git-66)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] virtio: Return correct errno for function init_vq's failure
2016-06-27 2:09 [PATCH] virtio: Return correct errno for function init_vq's failure Minfei Huang
@ 2016-07-06 9:18 ` Cornelia Huck
2016-07-13 11:54 ` Minfei Huang
0 siblings, 1 reply; 4+ messages in thread
From: Cornelia Huck @ 2016-07-06 9:18 UTC (permalink / raw)
To: Minfei Huang, mst; +Cc: Minfei Huang, linux-kernel, virtualization
On Mon, 27 Jun 2016 10:09:18 +0800
Minfei Huang <mnghuan@gmail.com> wrote:
> The error number -ENOENT or 0 will be returned, if we can not allocate
> more memory in function init_vq. If host can support multiple virtual
> queues, and we fails to allocate necessary memory structures for vq,
> kernel may crash due to incorrect returning.
>
> To fix it, kernel will return correct value in init_vq.
>
> Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> Signed-off-by: Minfei Huang <minfei.hmf@alibaba-inc.com>
> ---
> drivers/block/virtio_blk.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 42758b5..40ecb2b 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -393,11 +393,10 @@ static int init_vq(struct virtio_blk *vblk)
> if (err)
> num_vqs = 1;
>
> + err = -ENOMEM;
> vblk->vqs = kmalloc(sizeof(*vblk->vqs) * num_vqs, GFP_KERNEL);
> - if (!vblk->vqs) {
> - err = -ENOMEM;
> + if (!vblk->vqs)
> goto out;
> - }
>
> names = kmalloc(sizeof(*names) * num_vqs, GFP_KERNEL);
> if (!names)
The error handling in this function looks horrible.
When mq was introduced, init_vq started mixing up several things:
- The mq feature is not available - which is not an error, and
therefore should not have any influence on the return code.
- One of the several memory allocations failed - only ->vqs gets
special treatment, however.
- The ->find_vqs callback failed.
Your patch fixes the code, but it is still very convoluted due to the
temporary arrays.
May it be worthwile to introduce a helper for setting up the virtqueues
where all virtqueues are essentially the same and just get a
consecutive number? Michael?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] virtio: Return correct errno for function init_vq's failure
2016-07-06 9:18 ` Cornelia Huck
@ 2016-07-13 11:54 ` Minfei Huang
2016-07-13 12:05 ` Cornelia Huck
0 siblings, 1 reply; 4+ messages in thread
From: Minfei Huang @ 2016-07-13 11:54 UTC (permalink / raw)
To: Cornelia Huck; +Cc: mst, Minfei Huang, linux-kernel, virtualization
On 07/06/16 at 11:18P, Cornelia Huck wrote:
> On Mon, 27 Jun 2016 10:09:18 +0800
> Minfei Huang <mnghuan@gmail.com> wrote:
>
> > The error number -ENOENT or 0 will be returned, if we can not allocate
> > more memory in function init_vq. If host can support multiple virtual
> > queues, and we fails to allocate necessary memory structures for vq,
> > kernel may crash due to incorrect returning.
> >
> > To fix it, kernel will return correct value in init_vq.
> The error handling in this function looks horrible.
>
> When mq was introduced, init_vq started mixing up several things:
> - The mq feature is not available - which is not an error, and
> therefore should not have any influence on the return code.
> - One of the several memory allocations failed - only ->vqs gets
> special treatment, however.
> - The ->find_vqs callback failed.
Yep. And without this patch, it is silent for boot failure. I think it
makes sense to let user notify about this failure.
>
> Your patch fixes the code, but it is still very convoluted due to the
> temporary arrays.
>
> May it be worthwile to introduce a helper for setting up the virtqueues
> where all virtqueues are essentially the same and just get a
> consecutive number? Michael?
>
Hmm, How about refactor this function to make it more readable, since we
do a lot of work in it.
I will post an update to refactor this function.
Thanks
Minfei
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] virtio: Return correct errno for function init_vq's failure
2016-07-13 11:54 ` Minfei Huang
@ 2016-07-13 12:05 ` Cornelia Huck
0 siblings, 0 replies; 4+ messages in thread
From: Cornelia Huck @ 2016-07-13 12:05 UTC (permalink / raw)
To: Minfei Huang; +Cc: mst, Minfei Huang, linux-kernel, virtualization
On Wed, 13 Jul 2016 19:54:00 +0800
Minfei Huang <mnghuan@gmail.com> wrote:
> On 07/06/16 at 11:18P, Cornelia Huck wrote:
> > On Mon, 27 Jun 2016 10:09:18 +0800
> > Minfei Huang <mnghuan@gmail.com> wrote:
> >
> > > The error number -ENOENT or 0 will be returned, if we can not allocate
> > > more memory in function init_vq. If host can support multiple virtual
> > > queues, and we fails to allocate necessary memory structures for vq,
> > > kernel may crash due to incorrect returning.
> > >
> > > To fix it, kernel will return correct value in init_vq.
> > The error handling in this function looks horrible.
> >
> > When mq was introduced, init_vq started mixing up several things:
> > - The mq feature is not available - which is not an error, and
> > therefore should not have any influence on the return code.
> > - One of the several memory allocations failed - only ->vqs gets
> > special treatment, however.
> > - The ->find_vqs callback failed.
>
> Yep. And without this patch, it is silent for boot failure. I think it
> makes sense to let user notify about this failure.
Agreed.
>
> >
> > Your patch fixes the code, but it is still very convoluted due to the
> > temporary arrays.
> >
> > May it be worthwile to introduce a helper for setting up the virtqueues
> > where all virtqueues are essentially the same and just get a
> > consecutive number? Michael?
> >
>
> Hmm, How about refactor this function to make it more readable, since we
> do a lot of work in it.
>
> I will post an update to refactor this function.
Anything to make this more readable probably helps :)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-13 12:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-27 2:09 [PATCH] virtio: Return correct errno for function init_vq's failure Minfei Huang
2016-07-06 9:18 ` Cornelia Huck
2016-07-13 11:54 ` Minfei Huang
2016-07-13 12:05 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).