From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 1050C37A2 for ; Thu, 7 Dec 2017 18:15:37 +0100 (CET) Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Dec 2017 09:15:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,373,1508828400"; d="scan'208";a="928084" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by orsmga008.jf.intel.com with ESMTP; 07 Dec 2017 09:15:35 -0800 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.180]) by IRSMSX103.ger.corp.intel.com ([169.254.3.49]) with mapi id 14.03.0319.002; Thu, 7 Dec 2017 17:15:34 +0000 From: "Van Haaren, Harry" To: "Eads, Gage" , "dev@dpdk.org" CC: "jerin.jacob@caviumnetworks.com" , "Richardson, Bruce" , "hemant.agrawal@nxp.com" , "nipun.gupta@nxp.com" , "santosh.shukla@caviumnetworks.com" , "pbhagavatula@caviumnetworks.com" Thread-Topic: [PATCH 2/2] event/sw: use dynamically-sized IQs Thread-Index: AQHTaYiqxxfVOWUinESckeS2JLwVc6M4IqGw Date: Thu, 7 Dec 2017 17:15:35 +0000 Message-ID: References: <1512011314-19682-1-git-send-email-gage.eads@intel.com> <1512011314-19682-2-git-send-email-gage.eads@intel.com> In-Reply-To: <1512011314-19682-2-git-send-email-gage.eads@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZGEzOGViYWMtMjE3ZC00NGE1LWI0ODEtYmM1ZjU0OTk1MDJlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX1BVQkxJQyJ9XX1dfSwiU3ViamVjdExhYmVscyI6W10sIlRNQ1ZlcnNpb24iOiIxNi41LjkuMyIsIlRydXN0ZWRMYWJlbEhhc2giOiJuQUFEV1hQVmdcL2hrenZzNmJydDF4UnFDc0VKRjgwR2o3WUpSdkQrcnNCUT0ifQ== x-ctpclassification: CTP_PUBLIC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 2/2] event/sw: use dynamically-sized IQs 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: Thu, 07 Dec 2017 17:15:38 -0000 > From: Eads, Gage > Sent: Thursday, November 30, 2017 3:09 AM > To: dev@dpdk.org > Cc: jerin.jacob@caviumnetworks.com; Van Haaren, Harry > ; Richardson, Bruce > ; hemant.agrawal@nxp.com; nipun.gupta@nxp.com= ; > santosh.shukla@caviumnetworks.com; pbhagavatula@caviumnetworks.com > Subject: [PATCH 2/2] event/sw: use dynamically-sized IQs >=20 > This commit introduces dynamically-sized IQs, by switching the underlying > data structure from a fixed-size ring to a linked list of queue 'chunks.' > This has a number of benefits: > - Certain corner cases were observed in which all of a pipeline's flows > could be pinned to one port for extended periods, effectively turning a > multi-core pipeline into single-core one. This was caused by an event > producer having a larger new_event_threshold than the IQ depth, and > injecting large numbers of packets that are ultimately backpressured in= a > worker's rx_ring, causing those packets' flows to be scheduled to that > port. > The dynamically sized IQ does not have this problem because each IQ can > grow large enough to store all the system's events, such that > backpressure will not reach the worker_ring. > - Slight performance improvement (~1-2%) in high throughput scenarios, > tested with eventdev_pipeline_sw_pmd. >=20 > This implementation has a small increase in the queue storage memory > footprint (~70KB). This commit also removes the iq_size xstat, which no > longer applies to this implementation. >=20 > Signed-off-by: Gage Eads Some review notes below - but nothing that needs changing. Acked-by: Harry van Haaren > diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c > - for (i =3D 0; i < SW_IQS_MAX; i++) { > - snprintf(buf, sizeof(buf), "q_%u_iq_%d", idx, i); > - qid->iq[i] =3D iq_ring_create(buf, socket_id); > - if (!qid->iq[i]) { > - SW_LOG_DBG("ring create failed"); > - goto cleanup; > - } > - } > + for (i =3D 0; i < SW_IQS_MAX; i++) > + iq_init(sw, &qid->iq[i]); Review notes; - It looks like error checking is being lost above, but in reality the iq_i= nit() cannot fail in this new scheme, provided that the pool of chunks is p= rovisioned correctly at configure() time (see provisioning below). No action required here - just noting this error-checking change. > + /* Number of chunks sized for worst-case spread of events across IQs */ > + num_chunks =3D ((SW_INFLIGHT_EVENTS_TOTAL/SW_EVS_PER_Q_CHUNK)+1) + > + sw->qid_count*SW_IQS_MAX*2; To verify the logic here; ((TOTAL / CHUNK_SIZE) + 1) is the main body of chunks. QID * IQ * 2 is there so each QID can hold a chunk, and a next pointer, for= each priority of IQ. add these two together, is the total number of chunks. Check :)