Message ID | 20200709134823.9176-1-l.wojciechow@partner.samsung.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 08138A0528; Thu, 9 Jul 2020 15:48:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D2B841E916; Thu, 9 Jul 2020 15:48:48 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 469971E913 for <dev@dpdk.org>; Thu, 9 Jul 2020 15:48:47 +0200 (CEST) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20200709134846euoutp0273018988303a946532177a696a86feae~gGbN4cmAY0823008230euoutp02Y for <dev@dpdk.org>; Thu, 9 Jul 2020 13:48:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20200709134846euoutp0273018988303a946532177a696a86feae~gGbN4cmAY0823008230euoutp02Y DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1594302526; bh=U06hjsPf/wEQS+inyhjHJJc0BPWjF9d6ao14gcUQfps=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FJyXdppVMczMcudTvGKMDBGIeJQ98LUEWPMf5e9yzhZLjNUu6okuVoUbAdMTTNiby NuU5c4qE/bFAjWDtRyCdgYwdSY3U40knT4GULEwP1s6el28Lz3dGGzDVuTanLN0x6E xlnylTYAXuGWa4x34OwSTxvlsX4+DuUB1+b665AA= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20200709134846eucas1p122402a23819940fb3890305b6c689312~gGbNskgih1087810878eucas1p1F; Thu, 9 Jul 2020 13:48:46 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id CC.9D.05997.E30270F5; Thu, 9 Jul 2020 14:48:46 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20200709134846eucas1p193d963c3f21f0d5c4985024b6d015042~gGbNWGwEi1110411104eucas1p1G; Thu, 9 Jul 2020 13:48:46 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20200709134846eusmtrp1a0330bd63cf9f6c9d00f998f0460ead0~gGbNVl1Sl1329113291eusmtrp1L; Thu, 9 Jul 2020 13:48:46 +0000 (GMT) X-AuditID: cbfec7f4-65dff7000000176d-4a-5f07203e9638 Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 60.9E.06017.E30270F5; Thu, 9 Jul 2020 14:48:46 +0100 (BST) Received: from localhost.localdomain (unknown [106.210.88.70]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20200709134845eusmtip2ccf72659ac47a3fdc16fbaeebc415d50~gGbNDqsg12268722687eusmtip2R; Thu, 9 Jul 2020 13:48:45 +0000 (GMT) From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> To: Cc: dev@dpdk.org, l.wojciechow@partner.samsung.com Date: Thu, 9 Jul 2020 15:48:19 +0200 Message-Id: <20200709134823.9176-1-l.wojciechow@partner.samsung.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200422214555.11837-1-l.wojciechow@partner.samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrCIsWRmVeSWpSXmKPExsWy7djP87p2CuzxBi1rpSzefdrOZPGsZx2j A5PHrwVLWT0OvtvDFMAUxWWTkpqTWZZapG+XwJXR3jaPseC1dMXLv32MDYwXRboYOTkkBEwk Fk5/x97FyMUhJLCCUWLPngMsEM4XRom+VZOgnM+MEru+L2ODabk69SwTiC0ksJxRouVeBFzR mv9/WUESbAK2EkdmfgWzRQRYJFZ+/84CYjMLGEm87J7I3MXIwSEsYCyxcYo7SJhFQFVi184T YCW8Ai4SE1+vZITYJS+xesMBZhCbU8BN4vTRFrCDJAR2sEksO/WTBaLIReLSwi4mCFtY4tXx LewQtozE6ck9UA3bGCWu/v7JCOHsZ5S43rsCqspa4vC/32wgFzELaEqs36UPEXaU+D/xKAtI WEKAT+LGW0GI+/kkJm2bzgwR5pXoaBOCqNaTeNozlRFm7Z+1T6BO85A4d3AHKyR8ZjJKbNx+ lHECo/wshGULGBlXMYqnlhbnpqcWG+WllusVJ+YWl+al6yXn525iBMb16X/Hv+xg3PUn6RCj AAejEg9vwl+2eCHWxLLiytxDjBIczEoivE5nT8cJ8aYkVlalFuXHF5XmpBYfYpTmYFES5zVe 9DJWSCA9sSQ1OzW1ILUIJsvEwSnVwLj2fGLZhsa50059unrqgLc0l0yFyJKjxe7m23u2coTs r7yS4fC5UeYp5y3WJyV3Ju1d2+/znPeLa7ikjaHEG6V/wYI8tb6NGzSvZ77rvR25zYRjU9+c Jcc337YxfS5ipOmZ6bG2Sqss27d5+750xU0T49gq9kzjX9zw8DgL94sbG52Som/rxSuxFGck GmoxFxUnAgDhsQAc5wIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrMLMWRmVeSWpSXmKPExsVy+t/xe7p2CuzxBvPvsFq8+7SdyeJZzzpG ByaPXwuWsnocfLeHKYApSs+mKL+0JFUhI7+4xFYp2tDCSM/Q0kLPyMRSz9DYPNbKyFRJ384m JTUnsyy1SN8uQS+jvW0eY8Fr6YqXf/sYGxgvinQxcnJICJhIXJ16lgnEFhJYyiixfn5GFyMH UFxG4sMlAYgSYYk/17rYuhi5gEo+Mkp0f1jGDpJgE7CVODLzKyuILSLAIrHy+3cWEJsZaObt eU1sIHOEBYwlNk5xBwmzCKhK7Np5AqyEV8BFYuLrlYwQ8+UlVm84wAxicwq4SZw+2sICcY6r xNfDrewTGPkWMDKsYhRJLS3OTc8tNtIrTswtLs1L10vOz93ECAywbcd+btnB2PUu+BCjAAej Eg9vwl+2eCHWxLLiytxDjBIczEoivE5nT8cJ8aYkVlalFuXHF5XmpBYfYjQFOmois5Rocj4w +PNK4g1NDc0tLA3Njc2NzSyUxHk7BA7GCAmkJ5akZqemFqQWwfQxcXBKNTCeEex9tPA682Op ZqdHMvvl7Hg1YpMPLd89Pfantyf7+8BXtz4vfO/I8nLlT4tn+bMlHv8r/6Ue/URAj49fjbPt l/+7k2X1Lv8PMO+a472Ece2RQ94zD/HxPDvxcx//v4hrc3YEiEl/ahJ5nRB9707V7P1mD65+ 2cxXFKHalM66O2VHXtB6kVc3lFiKMxINtZiLihMBcWS9V0YCAAA= X-CMS-MailID: 20200709134846eucas1p193d963c3f21f0d5c4985024b6d015042 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200709134846eucas1p193d963c3f21f0d5c4985024b6d015042 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200709134846eucas1p193d963c3f21f0d5c4985024b6d015042 References: <20200422214555.11837-1-l.wojciechow@partner.samsung.com> <CGME20200709134846eucas1p193d963c3f21f0d5c4985024b6d015042@eucas1p1.samsung.com> Subject: [dpdk-dev] [PATCH v3 0/4] introduce global debug flag X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series | introduce global debug flag | |
Message
Lukasz Wojciechowski
July 9, 2020, 1:48 p.m. UTC
This set of patches introduces a global rte_debug flag for dpdk. This will allow easy switch to debug build configuration using a single flag. In the debug mode a RTE_DEBUG macro is defined to 1 and for every enabled to be built library a RTE_DEBUG_{library name} and for every enabled to be built driver a RTE_DEBUG_{driver_class}_{driver_name} is also defined. These macros can be used to place a debug code inside #ifdef #endif clauses. The following requirements were discussed on the mailing list: 1) The global debug flag is required to enable all the sanity checks and validations that are normally not used due to performance reasons 2) The best option would be to have a single flag - not to introduce too many build options 3) This option should be separated from meson "debug" option (used for build with symbols) and can be called "rte_debug" 4) The currently existing DEBUG macros should not be replaced with a RTE_DEBUG macro. This would allow to still enable them using CFLAGS="-D..." to test a single module (library, driver). 5) Currently existing options' names should be standardized to RTE_DEBUG_{library/driver name}, so they can be automatically enabled when rte_debug is set. Standardized names would allow easy usage in other modules. 6) The debug functionality should be encapsulated in: if (rte_log_can_log(...)) { ... } for possibility to be filtered out in runtime. Because of the hot discussion of v1 version of patches, I limit the v2 version to mbuf library changes only, to see how it will impact the performance with rte_log_can_log usage and to get opinions. v3 contains mbuf performance tests, which might help dpdk developers community to decide if drop of performance related to rte_log_can_log can be accepted. If agreement is reached, next steps would be to follow changes in other libraries and drivers. --- v3: * Define RTE_DEBUG_* flags also for drivers * Bring back CONFIG option for librte_mbuf, but with new flag name * Add mbuf performance tests v2: * Use new meson option rte_debug instead of debug * Add standardized defines for built libraries * Limit patches to mbuf library (as a POC) * Use rte_log_can_log to wrap debug section Lukasz Wojciechowski (4): config: introduce global rte debug flag config: remove unused config flags mbuf: standardize library debug flag app/test: add mbuf perf tests app/test/Makefile | 1 + app/test/meson.build | 4 +- app/test/test_mbuf.c | 3 +- app/test/test_mbuf_perf.c | 273 +++++++++++++++++++++++++++++ config/common_base | 6 +- config/meson.build | 4 + doc/guides/prog_guide/mbuf_lib.rst | 2 +- drivers/baseband/meson.build | 1 + drivers/bus/meson.build | 1 + drivers/common/meson.build | 1 + drivers/compress/meson.build | 1 + drivers/crypto/meson.build | 1 + drivers/event/meson.build | 1 + drivers/mempool/meson.build | 1 + drivers/meson.build | 3 + drivers/net/meson.build | 1 + drivers/raw/meson.build | 1 + drivers/vdpa/meson.build | 1 + lib/librte_mbuf/rte_mbuf.h | 12 +- lib/meson.build | 4 + meson_options.txt | 2 + 21 files changed, 312 insertions(+), 12 deletions(-) create mode 100644 app/test/test_mbuf_perf.c
Comments
09/07/2020 15:48, Lukasz Wojciechowski: > This set of patches introduces a global rte_debug flag for dpdk. > This will allow easy switch to debug build configuration using a single > flag. In the debug mode a RTE_DEBUG macro is defined to 1 > and for every enabled to be built library a RTE_DEBUG_{library name} > and for every enabled to be built driver > a RTE_DEBUG_{driver_class}_{driver_name} is also defined. > These macros can be used to place a debug code > inside #ifdef #endif clauses. > > The following requirements were discussed on the mailing list: > 1) The global debug flag is required to enable all the sanity checks > and validations that are normally not used due to performance reasons > > 2) The best option would be to have a single flag - not to introduce > too many build options > > 3) This option should be separated from meson "debug" option > (used for build with symbols) and can be called "rte_debug" > > 4) The currently existing DEBUG macros should not be replaced with > a RTE_DEBUG macro. This would allow to still enable them using > CFLAGS="-D..." to test a single module (library, driver). Please can we clarify this point? You mean not replacing driver macros with the global RTE_DEBUG? But we agree (next point) to replace existing macros? > 5) Currently existing options' names should be standardized > to RTE_DEBUG_{library/driver name}, so they can be automatically enabled > when rte_debug is set. Standardized names would allow easy usage > in other modules. > > 6) The debug functionality should be encapsulated in: > if (rte_log_can_log(...)) { > ... > } > for possibility to be filtered out in runtime. > > > Because of the hot discussion of v1 version of patches, I limit > the v2 version to mbuf library changes only, to see how it will impact > the performance with rte_log_can_log usage and to get opinions. > > v3 contains mbuf performance tests, which might help dpdk developers > community to decide if drop of performance related to rte_log_can_log > can be accepted. > > If agreement is reached, next steps would be to follow changes > in other libraries and drivers. If I understand well, it makes no sense to apply this v3, without having the rest of the code converted?
On Sat, Jul 11, 2020 at 05:11:52PM +0200, Thomas Monjalon wrote: > 09/07/2020 15:48, Lukasz Wojciechowski: > > This set of patches introduces a global rte_debug flag for dpdk. > > This will allow easy switch to debug build configuration using a single > > flag. In the debug mode a RTE_DEBUG macro is defined to 1 > > and for every enabled to be built library a RTE_DEBUG_{library name} > > and for every enabled to be built driver > > a RTE_DEBUG_{driver_class}_{driver_name} is also defined. > > These macros can be used to place a debug code > > inside #ifdef #endif clauses. > > > > The following requirements were discussed on the mailing list: > > 1) The global debug flag is required to enable all the sanity checks > > and validations that are normally not used due to performance reasons > > > > 2) The best option would be to have a single flag - not to introduce > > too many build options > > > > 3) This option should be separated from meson "debug" option > > (used for build with symbols) and can be called "rte_debug" > > > > 4) The currently existing DEBUG macros should not be replaced with > > a RTE_DEBUG macro. This would allow to still enable them using > > CFLAGS="-D..." to test a single module (library, driver). > > Please can we clarify this point? > You mean not replacing driver macros with the global RTE_DEBUG? > But we agree (next point) to replace existing macros? > > > 5) Currently existing options' names should be standardized > > to RTE_DEBUG_{library/driver name}, so they can be automatically enabled > > when rte_debug is set. Standardized names would allow easy usage > > in other modules. > > > > 6) The debug functionality should be encapsulated in: > > if (rte_log_can_log(...)) { > > ... > > } > > for possibility to be filtered out in runtime. > > > > > > Because of the hot discussion of v1 version of patches, I limit > > the v2 version to mbuf library changes only, to see how it will impact > > the performance with rte_log_can_log usage and to get opinions. > > > > v3 contains mbuf performance tests, which might help dpdk developers > > community to decide if drop of performance related to rte_log_can_log > > can be accepted. > > > > If agreement is reached, next steps would be to follow changes > > in other libraries and drivers. > > If I understand well, it makes no sense to apply this v3, > without having the rest of the code converted? > I'd nearly take the opposite view, that merging this patchset is a pre-requisite to converting the rest of the code. By merging this now, it allows others to work in parallel on any conversion they want to do, rather than requiring it all to be part of the one patchset. On the other hand, it would be good to get more buy-in from maintainers that this does meet their requirements at this point (and if not what could be changed to make it meet them) Regards, /Bruce
W dniu 11.07.2020 o 17:11, Thomas Monjalon pisze: > 09/07/2020 15:48, Lukasz Wojciechowski: >> This set of patches introduces a global rte_debug flag for dpdk. >> This will allow easy switch to debug build configuration using a single >> flag. In the debug mode a RTE_DEBUG macro is defined to 1 >> and for every enabled to be built library a RTE_DEBUG_{library name} >> and for every enabled to be built driver >> a RTE_DEBUG_{driver_class}_{driver_name} is also defined. >> These macros can be used to place a debug code >> inside #ifdef #endif clauses. >> >> The following requirements were discussed on the mailing list: >> 1) The global debug flag is required to enable all the sanity checks >> and validations that are normally not used due to performance reasons >> >> 2) The best option would be to have a single flag - not to introduce >> too many build options >> >> 3) This option should be separated from meson "debug" option >> (used for build with symbols) and can be called "rte_debug" >> >> 4) The currently existing DEBUG macros should not be replaced with >> a RTE_DEBUG macro. This would allow to still enable them using >> CFLAGS="-D..." to test a single module (library, driver). > Please can we clarify this point? > You mean not replacing driver macros with the global RTE_DEBUG? > But we agree (next point) to replace existing macros? Yes, I'll try. This point was added after discussion about using a single RTE_DEBUG flag for all debugs. I think we all agreed that it will be better to keep separate flags for separate libraries or drivers instead of using only one RTE_DEBUG flag for all debugs. e.g. in current version of patches there is a flag RTE_DEBUG_MBUF for debugs related to librte_mbuf. This allows to enable only some debugs at compile time passing them to CFLAGS. >> 5) Currently existing options' names should be standardized >> to RTE_DEBUG_{library/driver name}, so they can be automatically enabled >> when rte_debug is set. Standardized names would allow easy usage >> in other modules. >> >> 6) The debug functionality should be encapsulated in: >> if (rte_log_can_log(...)) { >> ... >> } >> for possibility to be filtered out in runtime. >> >> >> Because of the hot discussion of v1 version of patches, I limit >> the v2 version to mbuf library changes only, to see how it will impact >> the performance with rte_log_can_log usage and to get opinions. >> >> v3 contains mbuf performance tests, which might help dpdk developers >> community to decide if drop of performance related to rte_log_can_log >> can be accepted. >> >> If agreement is reached, next steps would be to follow changes >> in other libraries and drivers. > If I understand well, it makes no sense to apply this v3, > without having the rest of the code converted? > > > >
W dniu 13.07.2020 o 11:04, Bruce Richardson pisze: > On Sat, Jul 11, 2020 at 05:11:52PM +0200, Thomas Monjalon wrote: >> 09/07/2020 15:48, Lukasz Wojciechowski: >>> This set of patches introduces a global rte_debug flag for dpdk. >>> This will allow easy switch to debug build configuration using a single >>> flag. In the debug mode a RTE_DEBUG macro is defined to 1 >>> and for every enabled to be built library a RTE_DEBUG_{library name} >>> and for every enabled to be built driver >>> a RTE_DEBUG_{driver_class}_{driver_name} is also defined. >>> These macros can be used to place a debug code >>> inside #ifdef #endif clauses. >>> >>> The following requirements were discussed on the mailing list: >>> 1) The global debug flag is required to enable all the sanity checks >>> and validations that are normally not used due to performance reasons >>> >>> 2) The best option would be to have a single flag - not to introduce >>> too many build options >>> >>> 3) This option should be separated from meson "debug" option >>> (used for build with symbols) and can be called "rte_debug" >>> >>> 4) The currently existing DEBUG macros should not be replaced with >>> a RTE_DEBUG macro. This would allow to still enable them using >>> CFLAGS="-D..." to test a single module (library, driver). >> Please can we clarify this point? >> You mean not replacing driver macros with the global RTE_DEBUG? >> But we agree (next point) to replace existing macros? >> >>> 5) Currently existing options' names should be standardized >>> to RTE_DEBUG_{library/driver name}, so they can be automatically enabled >>> when rte_debug is set. Standardized names would allow easy usage >>> in other modules. >>> >>> 6) The debug functionality should be encapsulated in: >>> if (rte_log_can_log(...)) { >>> ... >>> } >>> for possibility to be filtered out in runtime. >>> >>> >>> Because of the hot discussion of v1 version of patches, I limit >>> the v2 version to mbuf library changes only, to see how it will impact >>> the performance with rte_log_can_log usage and to get opinions. >>> >>> v3 contains mbuf performance tests, which might help dpdk developers >>> community to decide if drop of performance related to rte_log_can_log >>> can be accepted. >>> >>> If agreement is reached, next steps would be to follow changes >>> in other libraries and drivers. >> If I understand well, it makes no sense to apply this v3, >> without having the rest of the code converted? >> > I'd nearly take the opposite view, that merging this patchset is a > pre-requisite to converting the rest of the code. By merging this now, it > allows others to work in parallel on any conversion they want to do, rather > than requiring it all to be part of the one patchset. > > On the other hand, it would be good to get more buy-in from maintainers > that this does meet their requirements at this point (and if not what could > be changed to make it meet them) > > Regards, > /Bruce I agree with Bruce. These patches are ready to apply. After they are introduced work can be parallelized and done for following libraries and drivers. However the mbuf library was chosen to verify performance of the solution. I published mbuf performance tests and the results for various compilation variants as a reply to former discussion, but there is no answer yet. The most interested person seemed to be Konstantin. Maybe we should wait for his opinion.
On Tue, 14 Jul 2020 00:39:43 +0200 Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> wrote: > W dniu 11.07.2020 o 17:11, Thomas Monjalon pisze: > > 09/07/2020 15:48, Lukasz Wojciechowski: > >> This set of patches introduces a global rte_debug flag for dpdk. > >> This will allow easy switch to debug build configuration using a single > >> flag. In the debug mode a RTE_DEBUG macro is defined to 1 > >> and for every enabled to be built library a RTE_DEBUG_{library name} > >> and for every enabled to be built driver > >> a RTE_DEBUG_{driver_class}_{driver_name} is also defined. > >> These macros can be used to place a debug code > >> inside #ifdef #endif clauses. > >> > >> The following requirements were discussed on the mailing list: > >> 1) The global debug flag is required to enable all the sanity checks > >> and validations that are normally not used due to performance reasons > >> > >> 2) The best option would be to have a single flag - not to introduce > >> too many build options > >> > >> 3) This option should be separated from meson "debug" option > >> (used for build with symbols) and can be called "rte_debug" > >> > >> 4) The currently existing DEBUG macros should not be replaced with > >> a RTE_DEBUG macro. This would allow to still enable them using > >> CFLAGS="-D..." to test a single module (library, driver). > > Please can we clarify this point? > > You mean not replacing driver macros with the global RTE_DEBUG? > > But we agree (next point) to replace existing macros? > Yes, I'll try. > > This point was added after discussion about using a single RTE_DEBUG > flag for all debugs. > I think we all agreed that it will be better to keep separate flags for > separate libraries or drivers instead of using only one RTE_DEBUG flag > for all debugs. > e.g. in current version of patches there is a flag RTE_DEBUG_MBUF for > debugs related to librte_mbuf. > This allows to enable only some debugs at compile time passing them to > CFLAGS. Has anyone investigated doing static branches in userspace? It might mean doing self-modifying code. That would allow always keeping/compiling in the debug code but at zero runtime cost.