From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 46783423B0; Wed, 11 Jan 2023 22:05:50 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 011B840FAE; Wed, 11 Jan 2023 22:05:50 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 71E9640A7D for ; Wed, 11 Jan 2023 22:05:48 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id AD7CF20B4524; Wed, 11 Jan 2023 13:05:47 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AD7CF20B4524 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1673471147; bh=bMjuINZHOScg8onQS038tpbPCQZP8XCt1Zrh1fhsda8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=VYnQhlUjyJlyMnEnl+Hunum8+vYTm1laBDh6HMGLBy3ESw8CwL+f5dThEXgqfm17D cIigwnK6kF8F7yiPpcjnP7zAVEcMlZcZoBUxb4epLDU7BszhvYO58UPyAxghlyv5M+ XNPOYHjU6YOToGHz8lOn6HmpmOuXPmOCXC4kMXzA= Date: Wed, 11 Jan 2023 13:05:47 -0800 From: Tyler Retzlaff To: Tomasz Duszynski Cc: "bruce.richardson@intel.com" , "mb@smartsharesystems.com" , "dev@dpdk.org" , "thomas@monjalon.net" , Jerin Jacob Kollanukkaran , "Ruifeng.Wang@arm.com" , "mattias.ronnblom@ericsson.com" , "zhoumin@loongson.cn" Subject: Re: [EXT] Re: [PATCH v5 0/4] add support for self monitoring Message-ID: <20230111210547.GA2134@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20221213104350.3218167-1-tduszynski@marvell.com> <20230110234642.1188550-1-tduszynski@marvell.com> <20230111003222.GA24460@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Jan 11, 2023 at 09:39:35AM +0000, Tomasz Duszynski wrote: > Hi Tyler, > > >-----Original Message----- > >From: Tyler Retzlaff > >Sent: Wednesday, January 11, 2023 1:32 AM > >To: Tomasz Duszynski ; bruce.richardson@intel.com; mb@smartsharesystems.com > >Cc: dev@dpdk.org; thomas@monjalon.net; Jerin Jacob Kollanukkaran ; > >mb@smartsharesystems.com; Ruifeng.Wang@arm.com; mattias.ronnblom@ericsson.com; zhoumin@loongson.cn > >Subject: [EXT] Re: [PATCH v5 0/4] add support for self monitoring > > > >External Email > > > >---------------------------------------------------------------------- > >hi, > > > >don't interpret this as an objection to the functionality but this looks like a clear example of > >something that doesn't belong in the EAL. has there been a discussion as to whether or not this > >should be in a separate library? > > No, I don't recall anybody having any concerns about the code placement. Rationale behind > making this part of eal was based on the fact that tracing itself is a part of eal and > since this was meant to be extension to tracing, code placement decision came out naturally. > > During development phase idea evolved a bit and what initially was supposed to be solely yet > another tracepoint become generic API to read pmu and tracepoint based on that. Which means > both can be used independently. > > That said, since this code has both platform agnostic and platform specific parts this can either be split into: > 1. library + eal platform code > 2. all under eal > > Either approach seems legit. Thoughts? > > > > >a basic test is whether or not an implementation exists or can be reasonably provided for all > >platforms and that isn't strictly evident here. red flag is to see yet more code being added > >conditionally compiled for a single platform. > > Even libs are not entirely pristine and have platform specific ifdefs lurking so not sure where > this red flag is coming from. i think red flag was probably the wrong term to use sorry for that. rather i should say it is an indicator that the api probably doesn't belong in the eal. fundamentally the purpose of the abstraction library is to relieve the application from having to do conditional compilation and/or execution for the subject apis coming from eal. including and exporting apis that work for only one platform is in direct contradiction. please explore adding this as a separate library, it is understood that there are tradeoffs involved. thanks!