On Mon, 24 May 2021 01:48:53 +0000, Guodeqing (A) <geffrey.guo@xxxxxxxxxx> wrote: > > > > -----Original Message----- > > From: Max Gurtovoy [mailto:mgurtovoy@xxxxxxxxxx] > > Sent: Sunday, May 23, 2021 15:25 > > To: Guodeqing (A) <geffrey.guo@xxxxxxxxxx>; mst@xxxxxxxxxx > > Cc: jasowang@xxxxxxxxxx; davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; > > virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH] virtio-net: fix the kzalloc/kfree mismatch problem > > > > > > On 5/22/2021 11:02 AM, guodeqing wrote: > > > If the virtio_net device does not suppurt the ctrl queue feature, the > > > vi->ctrl was not allocated, so there is no need to free it. > > > > you don't need this check. > > > > from kfree doc: > > > > "If @objp is NULL, no operation is performed." > > > > This is not a bug. I've set vi->ctrl to be NULL in case !vi->has_cvq. > > > > > yes, this is not a bug, the patch is just a optimization, because the vi->ctrl maybe > be freed which was not allocated, this may give people a misunderstanding. > Thanks. I think it may be enough to add a comment, and the code does not need to be modified. Thanks. > > > > > > Here I adjust the initialization sequence and the check of vi->has_cvq > > > to slove this problem. > > > > > > Fixes: 122b84a1267a ("virtio-net: don't allocate control_buf if not > > supported") > > > Signed-off-by: guodeqing <geffrey.guo@xxxxxxxxxx> > > > --- > > > drivers/net/virtio_net.c | 20 ++++++++++---------- > > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index > > > 9b6a4a875c55..894f894d3a29 100644 > > > --- a/drivers/net/virtio_net.c > > > +++ b/drivers/net/virtio_net.c > > > @@ -2691,7 +2691,8 @@ static void virtnet_free_queues(struct > > > virtnet_info *vi) > > > > > > kfree(vi->rq); > > > kfree(vi->sq); > > > - kfree(vi->ctrl); > > > + if (vi->has_cvq) > > > + kfree(vi->ctrl); > > > } > > > > > > static void _free_receive_bufs(struct virtnet_info *vi) @@ -2870,13 > > > +2871,6 @@ static int virtnet_alloc_queues(struct virtnet_info *vi) > > > { > > > int i; > > > > > > - if (vi->has_cvq) { > > > - vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL); > > > - if (!vi->ctrl) > > > - goto err_ctrl; > > > - } else { > > > - vi->ctrl = NULL; > > > - } > > > vi->sq = kcalloc(vi->max_queue_pairs, sizeof(*vi->sq), GFP_KERNEL); > > > if (!vi->sq) > > > goto err_sq; > > > @@ -2884,6 +2878,12 @@ static int virtnet_alloc_queues(struct > > virtnet_info *vi) > > > if (!vi->rq) > > > goto err_rq; > > > > > > + if (vi->has_cvq) { > > > + vi->ctrl = kzalloc(sizeof(*vi->ctrl), GFP_KERNEL); > > > + if (!vi->ctrl) > > > + goto err_ctrl; > > > + } > > > + > > > INIT_DELAYED_WORK(&vi->refill, refill_work); > > > for (i = 0; i < vi->max_queue_pairs; i++) { > > > vi->rq[i].pages = NULL; > > > @@ -2902,11 +2902,11 @@ static int virtnet_alloc_queues(struct > > > virtnet_info *vi) > > > > > > return 0; > > > > > > +err_ctrl: > > > + kfree(vi->rq); > > > err_rq: > > > kfree(vi->sq); > > > err_sq: > > > - kfree(vi->ctrl); > > > -err_ctrl: > > > return -ENOMEM; > > > } > > > > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
