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 9D6952C49 for ; Wed, 15 Mar 2017 11:06:20 +0100 (CET) Received: from pps.filterd (m0000700.ppops.net [127.0.0.1]) by mx0b-000f0801.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v2F9rxUN003326; Wed, 15 Mar 2017 03:06:20 -0700 Received: from brmwp-exmb12.corp.brocade.com ([208.47.132.227]) by mx0b-000f0801.pphosted.com with ESMTP id 2971mtrcsb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 15 Mar 2017 03:06:20 -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 04:06:17 -0600 To: Jan Blunck References: <1489444915-3660-1-git-send-email-ciwillia@brocade.com> <9c2a3831-11b8-2321-9dd8-0888a44bb876@brocade.com> CC: dev , From: "Charles (Chas) Williams" Message-ID: <00ca5e74-d3ef-ac13-d6b7-aaabcbb4c6fd@brocade.com> Date: Wed, 15 Mar 2017 06:06:15 -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-excas11.corp.brocade.com (10.70.36.102) 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-1703150078 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 10:06:20 -0000 On 03/15/2017 06:05 AM, Jan Blunck wrote: > On Wed, Mar 15, 2017 at 10:45 AM, Charles (Chas) Williams > wrote: >> >> >> 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. >> > > Other drivers (ixgbe, e1000, ...) always allocate based on the max > ring size too. Since the VMXNET3_RX_RING_MAX_SIZE is 4096 I don't > think it is a huge waste of memory. OK. BTW, the RX queues have the same issue.