From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-000f0801.pphosted.com (mx0b-000f0801.pphosted.com [67.231.152.113]) by dpdk.org (Postfix) with ESMTP id C61A31396 for ; Wed, 15 Mar 2017 10:45:45 +0100 (CET) Received: from pps.filterd (m0048192.ppops.net [127.0.0.1]) by mx0b-000f0801.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v2F9V0Tm020578; Wed, 15 Mar 2017 02:45:44 -0700 Received: from brmwp-exmb12.corp.brocade.com ([208.47.132.227]) by mx0b-000f0801.pphosted.com with ESMTP id 2971sf09ep-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 15 Mar 2017 02:45:44 -0700 Received: from [10.252.136.2] (10.252.136.2) by BRMWP-EXMB12.corp.brocade.com (172.16.59.130) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Wed, 15 Mar 2017 03:45:42 -0600 To: Jan Blunck References: <1489444915-3660-1-git-send-email-ciwillia@brocade.com> CC: dev , From: "Charles (Chas) Williams" Message-ID: <9c2a3831-11b8-2321-9dd8-0888a44bb876@brocade.com> Date: Wed, 15 Mar 2017 05:45:41 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: hq1wp-excas13.corp.brocade.com (10.70.36.103) To BRMWP-EXMB12.corp.brocade.com (172.16.59.130) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-03-15_03:, , signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703150071 Subject: Re: [dpdk-dev] [PATCH] net/vmxnet3: fix queue size changes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Mar 2017 09:45:46 -0000 On 03/15/2017 04:18 AM, Jan Blunck wrote: > On Tue, Mar 14, 2017 at 5:38 PM, Charles (Chas) Williams > wrote: >> >> >> On 03/14/2017 12:11 PM, Jan Blunck wrote: >>> >>> On Mon, Mar 13, 2017 at 11:41 PM, Charles (Chas) Williams >>> wrote: >>>> >>>> If the user reconfigures the queues size, then the previosly allocated >>>> memzone may potentially be too small. Instead, always free the old >>>> memzone and allocate a new one. >>>> >>>> Fixes: dfaff37fc46d ("vmxnet3: import new vmxnet3 poll mode driver >>>> implementation") >>>> >>>> Signed-off-by: Chas Williams >>>> --- >>>> drivers/net/vmxnet3/vmxnet3_rxtx.c | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c >>>> b/drivers/net/vmxnet3/vmxnet3_rxtx.c >>>> index 6649c3f..104e040 100644 >>>> --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c >>>> +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c >>>> @@ -893,8 +893,8 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf >>>> **rx_pkts, uint16_t nb_pkts) >>>> >>>> /* >>>> * Create memzone for device rings. malloc can't be used as the physical >>>> address is >>>> - * needed. If the memzone is already created, then this function returns >>>> a ptr >>>> - * to the old one. >>>> + * needed. If the memzone already exists, we free it since it may have >>>> been created >>>> + * with a different size. >>>> */ >>>> static const struct rte_memzone * >>>> ring_dma_zone_reserve(struct rte_eth_dev *dev, const char *ring_name, >>>> @@ -909,7 +909,7 @@ ring_dma_zone_reserve(struct rte_eth_dev *dev, const >>>> char *ring_name, >>>> >>>> mz = rte_memzone_lookup(z_name); >>>> if (mz) >>>> - return mz; >>>> + rte_memzone_free(mz); >>>> >>>> return rte_memzone_reserve_aligned(z_name, ring_size, >>>> socket_id, 0, >>>> VMXNET3_RING_BA_ALIGN); >>> >>> >>> Chas, >>> >>> Thanks for hunting this one down. Wouldn't the rte_memzone_free() >>> better fit into vmxnet3_cmd_ring_release() ? >> >> >> I don't care which way it goes. I just did what is basically done in >> gpa_zone_reserve() to match the "style". Tracking the current ring size >> and avoiding reallocating a potentially large chunk of memory seems like >> a better idea. >> >>> Also the ring_dma_zone_reserve() could get replaced by >>> rte_eth_dma_zone_reserve() (see also >> >> >> Yes, it probably should get changed to that along with tracking the size. > > Why don't we always allocate VMXNET3_RX_RING_MAX_SIZE entries? That > way we don't need to reallocate on a later queue setup change? That's using more memory than it needs to use. It might break someone's application that already runs in some tightly constrained machine. Failing to shrink the memzone isn't likely to break anything since they have (apparently) already dealt with having less memory available before switching to a smaller queue size. Still, it can be a matter for another day.