[{"id":2936527,"web_url":"http://patchwork.ozlabs.org/comment/2936527/","msgid":"<0b3c985d-d479-a554-4fe2-bfe94fc74070@redhat.com>","list_archive_url":null,"date":"2022-07-21T09:13:49","subject":"Re: [PATCH v12 08/40] virtio_ring: split: extract the logic of alloc\n queue","submitter":{"id":5225,"url":"http://patchwork.ozlabs.org/api/people/5225/","name":"Jason Wang","email":"jasowang@redhat.com"},"content":"在 2022/7/20 11:04, Xuan Zhuo 写道:\n> Separate the logic of split to create vring queue.\n>\n> This feature is required for subsequent virtuqueue reset vring.\n>\n> Signed-off-by: Xuan Zhuo \n> ---\n> drivers/virtio/virtio_ring.c | 68 ++++++++++++++++++++++--------------\n> 1 file changed, 42 insertions(+), 26 deletions(-)\n>\n> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c\n> index c94c5461e702..c7971438bb2c 100644\n> --- a/drivers/virtio/virtio_ring.c\n> +++ b/drivers/virtio/virtio_ring.c\n> @@ -950,28 +950,19 @@ static void vring_free_split(struct vring_virtqueue_split *vring_split,\n> \tkfree(vring_split->desc_extra);\n> }\n> \n> -static struct virtqueue *vring_create_virtqueue_split(\n> -\tunsigned int index,\n> -\tunsigned int num,\n> -\tunsigned int vring_align,\n> -\tstruct virtio_device *vdev,\n> -\tbool weak_barriers,\n> -\tbool may_reduce_num,\n> -\tbool context,\n> -\tbool (*notify)(struct virtqueue *),\n> -\tvoid (*callback)(struct virtqueue *),\n> -\tconst char *name)\n> +static int vring_alloc_queue_split(struct vring_virtqueue_split *vring_split,\n> +\t\t\t\t struct virtio_device *vdev,\n> +\t\t\t\t u32 num,\n> +\t\t\t\t unsigned int vring_align,\n> +\t\t\t\t bool may_reduce_num)\n> {\n> -\tstruct virtqueue *vq;\n> \tvoid *queue = NULL;\n> \tdma_addr_t dma_addr;\n> -\tsize_t queue_size_in_bytes;\n> -\tstruct vring vring;\n> \n> \t/* We assume num is a power of 2. */\n> \tif (num & (num - 1)) {\n> \t\tdev_warn(&vdev->dev, \"Bad virtqueue length %u\\n\", num);\n> -\t\treturn NULL;\n> +\t\treturn -EINVAL;\n> \t}\n> \n> \t/* TODO: allocate each queue chunk individually */\n> @@ -982,11 +973,11 @@ static struct virtqueue *vring_create_virtqueue_split(\n> \t\tif (queue)\n> \t\t\tbreak;\n> \t\tif (!may_reduce_num)\n> -\t\t\treturn NULL;\n> +\t\t\treturn -ENOMEM;\n> \t}\n> \n> \tif (!num)\n> -\t\treturn NULL;\n> +\t\treturn -ENOMEM;\n> \n> \tif (!queue) {\n> \t\t/* Try to get a single page. You are my only hope! */\n> @@ -994,21 +985,46 @@ static struct virtqueue *vring_create_virtqueue_split(\n> \t\t\t\t\t &dma_addr, GFP_KERNEL|__GFP_ZERO);\n> \t}\n> \tif (!queue)\n> -\t\treturn NULL;\n> +\t\treturn -ENOMEM;\n> +\n> +\tvring_init(&vring_split->vring, num, queue, vring_align);\n> \n> -\tqueue_size_in_bytes = vring_size(num, vring_align);\n> -\tvring_init(&vring, num, queue, vring_align);\n> +\tvring_split->queue_dma_addr = dma_addr;\n> +\tvring_split->queue_size_in_bytes = vring_size(num, vring_align);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static struct virtqueue *vring_create_virtqueue_split(\n> +\tunsigned int index,\n> +\tunsigned int num,\n> +\tunsigned int vring_align,\n> +\tstruct virtio_device *vdev,\n> +\tbool weak_barriers,\n> +\tbool may_reduce_num,\n> +\tbool context,\n> +\tbool (*notify)(struct virtqueue *),\n> +\tvoid (*callback)(struct virtqueue *),\n> +\tconst char *name)\n> +{\n> +\tstruct vring_virtqueue_split vring_split = {};\n> +\tstruct virtqueue *vq;\n> +\tint err;\n> +\n> +\terr = vring_alloc_queue_split(&vring_split, vdev, num, vring_align,\n> +\t\t\t\t may_reduce_num);\n> +\tif (err)\n> +\t\treturn NULL;\n> \n> -\tvq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,\n> -\t\t\t\t notify, callback, name);\n> +\tvq = __vring_new_virtqueue(index, vring_split.vring, vdev, weak_barriers,\n> +\t\t\t\t context, notify, callback, name);\n> \tif (!vq) {\n> -\t\tvring_free_queue(vdev, queue_size_in_bytes, queue,\n> -\t\t\t\t dma_addr);\n> +\t\tvring_free_split(&vring_split, vdev);\n> \t\treturn NULL;\n> \t}\n> \n> -\tto_vvq(vq)->split.queue_dma_addr = dma_addr;\n> -\tto_vvq(vq)->split.queue_size_in_bytes = queue_size_in_bytes;\n> +\tto_vvq(vq)->split.queue_dma_addr = vring_split.queue_dma_addr;\n> +\tto_vvq(vq)->split.queue_size_in_bytes = vring_split.queue_size_in_bytes;\n\n\nThis still seems a little bit redundant since the current logic is a \nlittle bit complicated since the vq->split is not initialized in a \nsingle place.\n\nI wonder if it's better to:\n\nvring_alloc_queue_split()\nvring_alloc_desc_extra() (reorder to make patch 9 come first)\n\nthen we can simply assign vring_split to vq->split in \n__vring_new_virtqueue() since it has:\n\n     vq->split.queue_dma_addr = 0;\n     vq->split.queue_size_in_bytes = 0;\n\n     vq->split.vring = vring;\n     vq->split.avail_flags_shadow = 0;\n     vq->split.avail_idx_shadow = 0;\n\nThis seems to simplify the logic and task of e.g \nvirtqueue_vring_attach_split() to a simple:\n\nvq->split= vring_split;\n\nAnd if this makes sense, we can do something similar to packed ring.\n\nThanks\n\n\n> \tto_vvq(vq)->we_own_ring = true;\n> \n> \treturn vq;","headers":{"Return-Path":"\n ","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=pass (2048-bit key;\n secure) header.d=lists.infradead.org header.i=@lists.infradead.org\n header.a=rsa-sha256 header.s=bombadil.20210309 header.b=B412SvSP;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=YuY6hov1;\n\tdkim-atps=neutral","ozlabs.org;\n spf=none (no SPF record) smtp.mailfrom=lists.infradead.org\n (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org;\n envelope-from=linux-um-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org;\n receiver=)","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=jasowang@redhat.com"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n [IPv6:2607:7c80:54:3::133])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby bilbo.ozlabs.org (Postfix) with ESMTPS id 4LpRhw0mkBz9s09\n\tfor ; Thu, 21 Jul 2022 19:14:24 +1000 (AEST)","from localhost ([::1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux))\n\tid 1oESG4-0035Pr-9b; Thu, 21 Jul 2022 09:14:12 +0000","from us-smtp-delivery-124.mimecast.com ([170.10.129.124])\n\tby bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux))\n\tid 1oESG1-0035L6-82\n\tfor linux-um@lists.infradead.org; Thu, 21 Jul 2022 09:14:10 +0000","from mail-pg1-f200.google.com (mail-pg1-f200.google.com\n [209.85.215.200]) by relay.mimecast.com with ESMTP with STARTTLS\n (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id\n us-mta-631-3pxyycsrNH6GK56Me4STaQ-1; Thu, 21 Jul 2022 05:14:02 -0400","by mail-pg1-f200.google.com with SMTP id\n x17-20020a631711000000b0041240801d34so655952pgl.17\n for ;\n Thu, 21 Jul 2022 02:14:02 -0700 (PDT)","from [10.72.12.47] ([209.132.188.80])\n by smtp.gmail.com with ESMTPSA id\n z28-20020aa7949c000000b0052516db7123sm1181543pfk.35.2022.07.21.02.13.51\n (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n Thu, 21 Jul 2022 02:14:00 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type:\n\tContent-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive:\n\tList-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject:\n\tMIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=MLoR3WeNEtwnHWUEPl7K4gG4kBq7vFnI7mTMA44Z09A=; b=B412SvSPJVHhc8\n\tydW32QxV+0Tr1lveSedFOPF7PftvIZspkHEVj+lnYublv5pfxX55WgGqbr1/mozPCIVlgPqMXtA5h\n\tsFmoMDOKvOC5Q4kjM1XkWS25lyqqjCVh6VxlUI92UEWEM0uKYmGQSdIPp1s3H0d1IbDORvHyp8kfL\n\tKcNVR43smL3L1Xgx6RgpBT8vuDfJua2Prf9WHorFjld+/k15R13Kx2E8M7TQA8a0WlnO7MFdwOhlI\n\tqmG1A21XW2WBaDAW/o6feFdMROr+jh0MKtndfDhLdIlHB3TGrwsUbGUkMGx48CXp+yBrKOc7zq9hj\n\tmnFQCcmIaEOzU9sO/Lkg==;","v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1658394847;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\t to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\t content-transfer-encoding:content-transfer-encoding:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=evkly4mzwl0OWnSDguyVCSXP+iuc3cHesZCTCuELO9c=;\n\tb=YuY6hov1brzRJK0PFq0RvULidQ7v0N9DQqq6GR2j3jfvCETpbK5z1pLIyjRKAPtrPYbTor\n\trBw538Fa+VZFBV6S+NB7jrNj8dvwSdjagNio8BKxeWdgSQMak6f5Gok4Jgcn9G1tybtkI0\n\tMlIWaofp+84W1dCl+f9p8eZQSfFLaOs="],"X-MC-Unique":"3pxyycsrNH6GK56Me4STaQ-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20210112;\n h=x-gm-message-state:message-id:date:mime-version:user-agent:subject\n :content-language:to:cc:references:from:in-reply-to\n :content-transfer-encoding;\n bh=evkly4mzwl0OWnSDguyVCSXP+iuc3cHesZCTCuELO9c=;\n b=L+xkGodEYCmOUmIuM8ZYBzzpRHkQIOgTdpo8u8visdta9YMn3JdSpgaLFpdw4PfcRi\n Z2zQ3GX4jnRFGh/K8I0uA4kRjm+cz4g7S8leglVhmjihzCmzBkX7fEXsDbYD1GrGuvOx\n jvmQ/aQVPXGz2T9B1qA77Y/dWwNoh0NQbfTaVQCnz9+6Oh+rez7oyEpYQSJoQ00mfy1A\n lZP1QY808j79B2K0805T+Rip/zxhqQQKhwLAdMF9WWFpGU9zeUBEmQnS2W5oSiexs3bM\n eCqCDhjLrlB4jQsED4yciE845o5XuUNTLJwAHbSgKeoiJz+PdFDylCW5sJXljRLxn433\n p0Jg==","X-Gm-Message-State":"AJIora/6eACLtfZstFsIaokuW0ubykZDbvoLcAEGEOpPsRhOuwEkZchV\n\tHQlsdVSAhqCKhCkAi1RimK2X4ILPyt7ljCE0nPhF9G/wweQuMcYW2QT/KAuTlIFpjfBNLnkMEfQ\n\tYMTTrYnZCa8blaeTestPYcXm6","X-Received":["by 2002:a17:90b:681:b0:1f2:147a:5e55 with SMTP id\n m1-20020a17090b068100b001f2147a5e55mr10137460pjz.159.1658394841815;\n Thu, 21 Jul 2022 02:14:01 -0700 (PDT)","by 2002:a17:90b:681:b0:1f2:147a:5e55 with SMTP id\n m1-20020a17090b068100b001f2147a5e55mr10137436pjz.159.1658394841532;\n Thu, 21 Jul 2022 02:14:01 -0700 (PDT)"],"X-Google-Smtp-Source":"\n AGRyM1vZd1H6ktLLHxg0oQu5yg8T5pv3EpQ1CpRpfXswCqn2eqimIJKu3rbioASMftx5YraJgvSUnw==","Message-ID":"<0b3c985d-d479-a554-4fe2-bfe94fc74070@redhat.com>","Date":"Thu, 21 Jul 2022 17:13:49 +0800","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)\n Gecko/20100101 Thunderbird/91.11.0","Subject":"Re: [PATCH v12 08/40] virtio_ring: split: extract the logic of alloc\n queue","To":"Xuan Zhuo ,\n virtualization@lists.linux-foundation.org","Cc":"Richard Weinberger ,\n Anton Ivanov ,\n Johannes Berg ,\n \"Michael S. Tsirkin\" , \"David S. Miller\"\n , Eric Dumazet ,\n Jakub Kicinski , Paolo Abeni ,\n Hans de Goede , Mark Gross ,\n Vadim Pasternak ,\n Bjorn Andersson ,\n Mathieu Poirier ,\n Cornelia Huck , Halil Pasic ,\n Eric Farman , Heiko Carstens ,\n Vasily Gorbik , Alexander Gordeev\n , Christian Borntraeger ,\n Sven Schnelle , Alexei Starovoitov ,\n Daniel Borkmann ,\n Jesper Dangaard Brouer ,\n John Fastabend ,\n Vincent Whitchurch ,\n linux-um@lists.infradead.org, netdev@vger.kernel.org,\n platform-driver-x86@vger.kernel.org, linux-remoteproc@vger.kernel.org,\n linux-s390@vger.kernel.org, kvm@vger.kernel.org, bpf@vger.kernel.org,\n kangjie.xu@linux.alibaba.com","References":"<20220720030436.79520-1-xuanzhuo@linux.alibaba.com>\n <20220720030436.79520-9-xuanzhuo@linux.alibaba.com>","From":"Jason Wang ","In-Reply-To":"<20220720030436.79520-9-xuanzhuo@linux.alibaba.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20220721_021409_382915_9BE89EA4 ","X-CRM114-Status":"GOOD ( 21.50 )","X-Spam-Score":"-1.6 (-)","X-Spam-Report":"=?unknown-8bit?q?Spam_detection_software=2C_running_on_the_sy?=\n\t=?unknown-8bit?q?stem_=22bombadil=2Einfradead=2Eorg=22=2C?=\n\t=?unknown-8bit?q?_has_NOT_identified_this_incoming_email_as_spam=2E__The_ori?=\n\t=?unknown-8bit?q?ginal?=\n\t=?unknown-8bit?q?_message_has_been_attached_to_this_so_you_can_view_it_or_la?=\n\t=?unknown-8bit?q?bel?=\n\t=?unknown-8bit?q?_similar_future_email=2E__If_you_have_any_questions=2C_see?=\n\t=?unknown-8bit?q?_the_administrator_of_that_system_for_details=2E?=\n\t=?unknown-8bit?q?_?=\n\t=?unknown-8bit?b?IENvbnRlbnQgcHJldmlldzogIOWcqCAyMDIyLzcvMjAgMTE6MDQsIFh1?=\n\t=?unknown-8bit?b?YW4gWmh1byDlhpnpgZM6ID4gU2VwYXJhdGUgdGhlIGxvZ2lj?=\n\t=?unknown-8bit?q?_of_split_to_create_vring_queue=2E_=3E_=3E_This_feature_is_?=\n\t=?unknown-8bit?q?required_for_subsequent?=\n\t=?unknown-8bit?q?_virtuqueue_reset_vring=2E_=3E_=3E_Signed-off-by=3A_Xuan_Zh?=\n\t=?unknown-8bit?q?uo_=3Cxuanzhuo=40l_=5B=2E=2E=2E=5D_?=\n\t=?unknown-8bit?q?_?=\n\t=?unknown-8bit?q?_Content_analysis_details=3A___=28-1=2E6_points=2C_5=2E0_re?=\n\t=?unknown-8bit?q?quired=29?=\n\t=?unknown-8bit?q?_?=\n\t=?unknown-8bit?q?_pts_rule_name______________description?=\n\t=?unknown-8bit?q?_----_----------------------_------------------------------?=\n\t=?unknown-8bit?q?--------------------?=\n\t=?unknown-8bit?q?_-0=2E7_RCVD=5FIN=5FDNSWL=5FLOW______RBL=3A_Sender_listed_a?=\n\t=?unknown-8bit?q?t_https=3A//www=2Ednswl=2Eorg/=2C?=\n\t=?unknown-8bit?q?_low_trust?=\n\t=?unknown-8bit?q?_=5B170=2E10=2E129=2E124_listed_in_list=2Ednswl=2Eorg=5D?=\n\t=?unknown-8bit?q?_0=2E0_SPF=5FHELO=5FNONE__________SPF=3A_HELO_does_not_publ?=\n\t=?unknown-8bit?q?ish_an_SPF_Record?=\n\t=?unknown-8bit?q?_0=2E0_SPF=5FNONE_______________SPF=3A_sender_does_not_publ?=\n\t=?unknown-8bit?q?ish_an_SPF_Record?=\n\t=?unknown-8bit?q?_-0=2E1_DKIM=5FVALID=5FAU__________Message_has_a_valid_DKIM?=\n\t=?unknown-8bit?q?_or_DK_signature_from?=\n\t=?unknown-8bit?q?_author=27s_domain?=\n\t=?unknown-8bit?q?_-0=2E1_DKIM=5FVALID_____________Message_has_at_least_one_v?=\n\t=?unknown-8bit?q?alid_DKIM_or_DK_signature?=\n\t=?unknown-8bit?q?_0=2E1_DKIM=5FSIGNED____________Message_has_a_DKIM_or_DK_si?=\n\t=?unknown-8bit?q?gnature=2C_not_necessarily?=\n\t=?unknown-8bit?q?_valid?=\n\t=?unknown-8bit?q?_-0=2E1_DKIM=5FVALID=5FEF__________Message_has_a_valid_DKIM?=\n\t=?unknown-8bit?q?_or_DK_signature_from?=\n\t=?unknown-8bit?q?_envelope-from_domain?=\n\t=?unknown-8bit?q?_-0=2E5_NICE=5FREPLY=5FA___________Looks_like_a_legit_reply?=\n\t=?unknown-8bit?q?_=28A=29?=\n\t=?unknown-8bit?q?_-0=2E2_DKIMWL=5FWL=5FHIGH_________DKIMwl=2Eorg_-_High_trus?=\n\t=?unknown-8bit?q?t_sender?=","X-BeenThere":"linux-um@lists.infradead.org","X-Mailman-Version":"2.1.34","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Sender":"\"linux-um\" ","Errors-To":"linux-um-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org"}},{"id":2937157,"web_url":"http://patchwork.ozlabs.org/comment/2937157/","msgid":"<1658461678.632858-2-xuanzhuo@linux.alibaba.com>","list_archive_url":null,"date":"2022-07-22T03:47:58","subject":"Re: [PATCH v12 08/40] virtio_ring: split: extract the logic of alloc\n queue","submitter":{"id":80519,"url":"http://patchwork.ozlabs.org/api/people/80519/","name":"Xuan Zhuo","email":"xuanzhuo@linux.alibaba.com"},"content":"On Thu, 21 Jul 2022 17:13:49 +0800, Jason Wang wrote:\n>\n> 在 2022/7/20 11:04, Xuan Zhuo 写道:\n> > Separate the logic of split to create vring queue.\n> >\n> > This feature is required for subsequent virtuqueue reset vring.\n> >\n> > Signed-off-by: Xuan Zhuo \n> > ---\n> > drivers/virtio/virtio_ring.c | 68 ++++++++++++++++++++++--------------\n> > 1 file changed, 42 insertions(+), 26 deletions(-)\n> >\n> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c\n> > index c94c5461e702..c7971438bb2c 100644\n> > --- a/drivers/virtio/virtio_ring.c\n> > +++ b/drivers/virtio/virtio_ring.c\n> > @@ -950,28 +950,19 @@ static void vring_free_split(struct vring_virtqueue_split *vring_split,\n> > \tkfree(vring_split->desc_extra);\n> > }\n> >\n> > -static struct virtqueue *vring_create_virtqueue_split(\n> > -\tunsigned int index,\n> > -\tunsigned int num,\n> > -\tunsigned int vring_align,\n> > -\tstruct virtio_device *vdev,\n> > -\tbool weak_barriers,\n> > -\tbool may_reduce_num,\n> > -\tbool context,\n> > -\tbool (*notify)(struct virtqueue *),\n> > -\tvoid (*callback)(struct virtqueue *),\n> > -\tconst char *name)\n> > +static int vring_alloc_queue_split(struct vring_virtqueue_split *vring_split,\n> > +\t\t\t\t struct virtio_device *vdev,\n> > +\t\t\t\t u32 num,\n> > +\t\t\t\t unsigned int vring_align,\n> > +\t\t\t\t bool may_reduce_num)\n> > {\n> > -\tstruct virtqueue *vq;\n> > \tvoid *queue = NULL;\n> > \tdma_addr_t dma_addr;\n> > -\tsize_t queue_size_in_bytes;\n> > -\tstruct vring vring;\n> >\n> > \t/* We assume num is a power of 2. */\n> > \tif (num & (num - 1)) {\n> > \t\tdev_warn(&vdev->dev, \"Bad virtqueue length %u\\n\", num);\n> > -\t\treturn NULL;\n> > +\t\treturn -EINVAL;\n> > \t}\n> >\n> > \t/* TODO: allocate each queue chunk individually */\n> > @@ -982,11 +973,11 @@ static struct virtqueue *vring_create_virtqueue_split(\n> > \t\tif (queue)\n> > \t\t\tbreak;\n> > \t\tif (!may_reduce_num)\n> > -\t\t\treturn NULL;\n> > +\t\t\treturn -ENOMEM;\n> > \t}\n> >\n> > \tif (!num)\n> > -\t\treturn NULL;\n> > +\t\treturn -ENOMEM;\n> >\n> > \tif (!queue) {\n> > \t\t/* Try to get a single page. You are my only hope! */\n> > @@ -994,21 +985,46 @@ static struct virtqueue *vring_create_virtqueue_split(\n> > \t\t\t\t\t &dma_addr, GFP_KERNEL|__GFP_ZERO);\n> > \t}\n> > \tif (!queue)\n> > -\t\treturn NULL;\n> > +\t\treturn -ENOMEM;\n> > +\n> > +\tvring_init(&vring_split->vring, num, queue, vring_align);\n> >\n> > -\tqueue_size_in_bytes = vring_size(num, vring_align);\n> > -\tvring_init(&vring, num, queue, vring_align);\n> > +\tvring_split->queue_dma_addr = dma_addr;\n> > +\tvring_split->queue_size_in_bytes = vring_size(num, vring_align);\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +static struct virtqueue *vring_create_virtqueue_split(\n> > +\tunsigned int index,\n> > +\tunsigned int num,\n> > +\tunsigned int vring_align,\n> > +\tstruct virtio_device *vdev,\n> > +\tbool weak_barriers,\n> > +\tbool may_reduce_num,\n> > +\tbool context,\n> > +\tbool (*notify)(struct virtqueue *),\n> > +\tvoid (*callback)(struct virtqueue *),\n> > +\tconst char *name)\n> > +{\n> > +\tstruct vring_virtqueue_split vring_split = {};\n> > +\tstruct virtqueue *vq;\n> > +\tint err;\n> > +\n> > +\terr = vring_alloc_queue_split(&vring_split, vdev, num, vring_align,\n> > +\t\t\t\t may_reduce_num);\n> > +\tif (err)\n> > +\t\treturn NULL;\n> >\n> > -\tvq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,\n> > -\t\t\t\t notify, callback, name);\n> > +\tvq = __vring_new_virtqueue(index, vring_split.vring, vdev, weak_barriers,\n> > +\t\t\t\t context, notify, callback, name);\n> > \tif (!vq) {\n> > -\t\tvring_free_queue(vdev, queue_size_in_bytes, queue,\n> > -\t\t\t\t dma_addr);\n> > +\t\tvring_free_split(&vring_split, vdev);\n> > \t\treturn NULL;\n> > \t}\n> >\n> > -\tto_vvq(vq)->split.queue_dma_addr = dma_addr;\n> > -\tto_vvq(vq)->split.queue_size_in_bytes = queue_size_in_bytes;\n> > +\tto_vvq(vq)->split.queue_dma_addr = vring_split.queue_dma_addr;\n> > +\tto_vvq(vq)->split.queue_size_in_bytes = vring_split.queue_size_in_bytes;\n>\n>\n> This still seems a little bit redundant since the current logic is a\n> little bit complicated since the vq->split is not initialized in a\n> single place.\n>\n> I wonder if it's better to:\n>\n> vring_alloc_queue_split()\n> vring_alloc_desc_extra() (reorder to make patch 9 come first)\n>\n> then we can simply assign vring_split to vq->split in\n> __vring_new_virtqueue() since it has:\n>\n>     vq->split.queue_dma_addr = 0;\n>     vq->split.queue_size_in_bytes = 0;\n>\n>     vq->split.vring = vring;\n>     vq->split.avail_flags_shadow = 0;\n>     vq->split.avail_idx_shadow = 0;\n>\n> This seems to simplify the logic and task of e.g\n> virtqueue_vring_attach_split() to a simple:\n>\n> vq->split= vring_split;\n\nThis does look simpler. The reason for not doing this is that the argument\naccepted by __vring_new_virtqueue() is \"struct vring\", and\n__vring_new_virtqueue() is an export symbol.\n\nI took a look, and the only external direct call to __vring_new_virtqueue is\nhere.\n\n\ttools/virtio/virtio_test.c\n\tstatic void vq_reset(struct vq_info *info, int num, struct virtio_device *vdev)\n\t{\n\t\tif (info->vq)\n\t\t\tvring_del_virtqueue(info->vq);\n\n\t\tmemset(info->ring, 0, vring_size(num, 4096));\n\t\tvring_init(&info->vring, num, info->ring, 4096);\n\t\tinfo->vq = __vring_new_virtqueue(info->idx, info->vring, vdev, true,\n\t\t\t\t\t\t false, vq_notify, vq_callback, \"test\");\n\t\tassert(info->vq);\n\t\tinfo->vq->priv = info;\n\t}\n\nI think this could be replaced with vring_new_virtqueue() so that we don't need\nto make __vring_new_virtqueue as an export function so we can make some\nmodifications to it.\n\nnit: vring_alloc_desc_extra() should not have to be extract from\n__vring_new_virtqueue() .\n\nThanks.\n\n>\n> And if this makes sense, we can do something similar to packed ring.\n>\n> Thanks\n>\n>\n> > \tto_vvq(vq)->we_own_ring = true;\n> >\n> > \treturn vq;\n>","headers":{"Return-Path":"\n ","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["bilbo.ozlabs.org;\n\tdkim=pass (2048-bit key;\n secure) header.d=lists.infradead.org header.i=@lists.infradead.org\n header.a=rsa-sha256 header.s=bombadil.20210309 header.b=wVoxx5Zp;\n\tdkim-atps=neutral","ozlabs.org;\n spf=none (no SPF record) smtp.mailfrom=lists.infradead.org\n (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org;\n envelope-from=linux-um-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org;\n receiver=)"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n [IPv6:2607:7c80:54:3::133])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby bilbo.ozlabs.org (Postfix) with ESMTPS id 4Lpwdc0D5pz9s1l\n\tfor ; Fri, 22 Jul 2022 13:58:09 +1000 (AEST)","from localhost ([::1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux))\n\tid 1oEjnZ-00HMTW-0h; Fri, 22 Jul 2022 03:57:57 +0000","from out30-57.freemail.mail.aliyun.com ([115.124.30.57])\n\tby bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux))\n\tid 1oEjnV-00HMS7-7y\n\tfor linux-um@lists.infradead.org; Fri, 22 Jul 2022 03:57:55 +0000","from localhost(mailfrom:xuanzhuo@linux.alibaba.com\n fp:SMTPD_---0VK3SFm4_1658462266)\n by smtp.aliyun-inc.com;\n Fri, 22 Jul 2022 11:57:47 +0800"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20210309; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:MIME-Version:List-Subscribe:List-Help:\n\tList-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:References:Cc:To:\n\tFrom:Date:Subject:Message-ID:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=V1d+FSLR0tY0uws8lUjqpUVAYQgGJpirqKT8kVfqmAQ=; b=wVoxx5Zpm5bjrk\n\tx75erxVJDZ7Mqip6SoMBo9rIUK03suId+35svydDWgY9bJ4/NCzynO2i4AOQ0hGcIRn3uvSjyE3H0\n\tytmhjjJhIzMyDeOd1vMvC4mGpn3/vjOlDOKv4QpJ40o9P0AulUp2IC/bD1izarnPtQweGk5iOaKxU\n\tncetQFlMMvwQCNq+Lq5w/LolLq259uWu1yFXnTjuxuv+MB7ccfaM2x+92hrkMuoU4pRvlvOqg4I73\n\tNtEv5teBr+WGpBSAMtUr5Tz6no6zyVu1oHXkVJNzFAJ7cR3IGi2u597Wv6kccAziPAqygeIJ6VjkN\n\thJVhsTKZPs0ElB0dZwbA==;","X-Alimail-AntiSpam":"\n AC=PASS;BC=-1|-1;BR=01201311R121e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04400;MF=xuanzhuo@linux.alibaba.com;NM=1;PH=DS;RN=36;SR=0;TI=SMTPD_---0VK3SFm4_1658462266;","Message-ID":"<1658461678.632858-2-xuanzhuo@linux.alibaba.com>","Subject":"Re: [PATCH v12 08/40] virtio_ring: split: extract the logic of alloc\n queue","Date":"Fri, 22 Jul 2022 11:47:58 +0800","From":"Xuan Zhuo ","To":"Jason Wang ","Cc":"Richard Weinberger ,\n Anton Ivanov ,\n Johannes Berg ,\n \"Michael S. Tsirkin\" ,\n \"David S. Miller\" ,\n Eric Dumazet ,\n Jakub Kicinski ,\n Paolo Abeni ,\n Hans de Goede ,\n Mark Gross ,\n Vadim Pasternak ,\n Bjorn Andersson ,\n Mathieu Poirier ,\n Cornelia Huck ,\n Halil Pasic ,\n Eric Farman ,\n Heiko Carstens ,\n Vasily Gorbik ,\n Alexander Gordeev ,\n Christian Borntraeger ,\n Sven Schnelle ,\n Alexei Starovoitov ,\n Daniel Borkmann ,\n Jesper Dangaard Brouer ,\n John Fastabend ,\n Vincent Whitchurch ,\n linux-um@lists.infradead.org,\n netdev@vger.kernel.org,\n platform-driver-x86@vger.kernel.org,\n linux-remoteproc@vger.kernel.org,\n linux-s390@vger.kernel.org,\n kvm@vger.kernel.org,\n bpf@vger.kernel.org,\n kangjie.xu@linux.alibaba.com,\n virtualization@lists.linux-foundation.org","References":"<20220720030436.79520-1-xuanzhuo@linux.alibaba.com>\n <20220720030436.79520-9-xuanzhuo@linux.alibaba.com>\n <0b3c985d-d479-a554-4fe2-bfe94fc74070@redhat.com>","In-Reply-To":"<0b3c985d-d479-a554-4fe2-bfe94fc74070@redhat.com>","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20220721_205753_489410_255983FF ","X-CRM114-Status":"GOOD ( 24.80 )","X-Spam-Score":"-8.0 (--------)","X-Spam-Report":"=?unknown-8bit?q?Spam_detection_software=2C_running_on_the_sy?=\n\t=?unknown-8bit?q?stem_=22bombadil=2Einfradead=2Eorg=22=2C?=\n\t=?unknown-8bit?q?_has_NOT_identified_this_incoming_email_as_spam=2E__The_ori?=\n\t=?unknown-8bit?q?ginal?=\n\t=?unknown-8bit?q?_message_has_been_attached_to_this_so_you_can_view_it_or_la?=\n\t=?unknown-8bit?q?bel?=\n\t=?unknown-8bit?q?_similar_future_email=2E__If_you_have_any_questions=2C_see?=\n\t=?unknown-8bit?q?_the_administrator_of_that_system_for_details=2E?=\n\t=?unknown-8bit?q?_?=\n\t=?unknown-8bit?q?_Content_preview=3A__On_Thu=2C_21_Jul_2022_17=3A13=3A49_+08?=\n\t=?unknown-8bit?q?00=2C_Jason_Wang_=3Cjasowang=40redhat=2Ecom=3E?=\n\t=?unknown-8bit?b?IHdyb3RlOiA+ID4g5ZyoIDIwMjIvNy8yMCAxMTowNCwgWHVhbiBaaHVv?=\n\t=?unknown-8bit?b?IOWGmemBkzogPiA+IFNlcGFyYXRlIHRoZSBsb2dpYw==?=\n\t=?unknown-8bit?q?_of_split_to_create_vring_queue=2E_=3E_=3E_=3E_=3E_This_fea?=\n\t=?unknown-8bit?q?ture_is_re_=5B=2E=2E=2E=5D_?=\n\t=?unknown-8bit?q?_?=\n\t=?unknown-8bit?q?_Content_analysis_details=3A___=28-8=2E0_points=2C_5=2E0_re?=\n\t=?unknown-8bit?q?quired=29?=\n\t=?unknown-8bit?q?_?=\n\t=?unknown-8bit?q?_pts_rule_name______________description?=\n\t=?unknown-8bit?q?_----_----------------------_------------------------------?=\n\t=?unknown-8bit?q?--------------------?=\n\t=?unknown-8bit?q?_-0=2E0_RCVD=5FIN=5FDNSWL=5FNONE_____RBL=3A_Sender_listed_a?=\n\t=?unknown-8bit?q?t_https=3A//www=2Ednswl=2Eorg/=2C?=\n\t=?unknown-8bit?q?_no_trust?=\n\t=?unknown-8bit?q?_=5B115=2E124=2E30=2E57_listed_in_list=2Ednswl=2Eorg=5D?=\n\t=?unknown-8bit?q?_-0=2E0_SPF=5FPASS_______________SPF=3A_sender_matches_SPF_?=\n\t=?unknown-8bit?q?record?=\n\t=?unknown-8bit?q?_0=2E0_SPF=5FHELO=5FNONE__________SPF=3A_HELO_does_not_publ?=\n\t=?unknown-8bit?q?ish_an_SPF_Record?=\n\t=?unknown-8bit?q?_-7=2E5_USER=5FIN=5FDEF=5FSPF=5FWL_____From=3A_address_is_i?=\n\t=?unknown-8bit?q?n_the_default_SPF?=\n\t=?unknown-8bit?q?_white-list?=\n\t=?unknown-8bit?q?_-0=2E5_ENV=5FAND=5FHDR=5FSPF=5FMATCH__Env_and_Hdr_From_use?=\n\t=?unknown-8bit?q?d_in_default_SPF_WL?=\n\t=?unknown-8bit?q?_Match?=\n\t=?unknown-8bit?q?_0=2E0_UNPARSEABLE=5FRELAY______Informational=3A_message_ha?=\n\t=?unknown-8bit?q?s_unparseable_relay?=\n\t=?unknown-8bit?q?_lines?=","X-BeenThere":"linux-um@lists.infradead.org","X-Mailman-Version":"2.1.34","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Sender":"\"linux-um\" ","Errors-To":"linux-um-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org"}}]