From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM01-BN3-obe.outbound.protection.outlook.com (mail-bn3nam01on0060.outbound.protection.outlook.com [104.47.33.60]) by dpdk.org (Postfix) with ESMTP id 30EE71B1B2 for ; Wed, 11 Oct 2017 07:23:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=CAVIUMNETWORKS.onmicrosoft.com; s=selector1-cavium-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=daTnrRHLT2manu4Lqj39D5lsIT9fM/cvXuJ17UQfJ8g=; b=lCq+tCOB5V1P4Wj5renPXDTkovH2PIEz9aReIPzKxTkujmOB9QH/1f63pljuYe9ww2JyBE8Hi1nWZCgRhxKG4rp69btWpdyYVUwbxY0kf8b+05P+O9jLxwGmHNYkH6xCaUMOfqYutbKliGBwmTwP7+itwWziWHknDwO/hnikZHE= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Santosh.Shukla@cavium.com; Received: from [192.168.0.105] (103.76.56.167) by MWHPR07MB3101.namprd07.prod.outlook.com (10.172.95.7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.77.7; Wed, 11 Oct 2017 05:23:40 +0000 To: Shreyansh Jain , Thomas Monjalon References: <20171010070155.17412-1-shreyansh.jain@nxp.com> <2102754.igz1rhL23C@xps> <7387c315-fd0f-a5a2-fdd9-1ae9aed69e9b@caviumnetworks.com> <1821646.02OIGBtUFx@xps> <421a112e-69f1-b8bc-c863-cd08dad5cc7f@nxp.com> Cc: dev@dpdk.org, ferruh.yigit@intel.com, hemant.agrawal@nxp.com From: santosh Message-ID: Date: Wed, 11 Oct 2017 10:53:22 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <421a112e-69f1-b8bc-c863-cd08dad5cc7f@nxp.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Originating-IP: [103.76.56.167] X-ClientProxiedBy: BMXPR01CA0014.INDPRD01.PROD.OUTLOOK.COM (10.174.214.152) To MWHPR07MB3101.namprd07.prod.outlook.com (10.172.95.7) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 621954fe-5b53-4fab-0b1a-08d510683cdd X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254152)(2017052603199)(201703131423075)(201703031133081)(201702281549075); SRVR:MWHPR07MB3101; X-Microsoft-Exchange-Diagnostics: 1; MWHPR07MB3101; 3:OWnBVy6XG8tt4wZUjY3/WujvHfsVCFsXfjm4rNamcRJX/p30UoZsWD3Habh+zLMHkEbjXg3mSF5wS4JJKYuOA5EXbwcHX8Wl8VppCsBkOwaNys2LlJ3hLsWZtZ915MBZ+6/Pd0tC6towb7eXuYqCjDnaoM/G/xIyid5jhrSrwkxMaPZcgruS7uw3NL5Q/9fSRz9omsu9FIWDjVwxw8/ak847YuM1/HuQjPzTCF2LrxbY8To5v4mc4ci9xmYvQgBp; 25:VZ9P+x7cEoczTSMCoGDTn9+btG3moRGJvSTvPIakzo2Twe7Y+9cFgcS0GkQmAu5Z1q0fSyG4Pn0lCK9DTqQJGCedu0qjNNSxRtq3inAJP/DQiKsZesDYmsdwTtwR4P91suHYu2eLsxh2vtsrRg6sM1Ojb12M73O2QZaccyf5A7tbxB2qRBCewHH66bwwZC5L3t+7clv6YgzgVnmWgq9aj+jKfShabPxd7Fp5yX6xW6neLWK1QnZDQTU62O6xGjVZ+QvT7ZVcs+VjcXqLz9g1+4p0eCWzDhlDuUn0SdC68GJGFtES4FDIscTBkF8nXM66TqGA6CweB1GK3XzZKcVl3Q==; 31:K/bZq2HidgSO/82+ja7ZqvHM55QkFC4SAjTySOijET5Bp+GXUH9/IE6UduOUmtEM8f6VjyNrLooiiZFX69PZo0QrZ1zJxkr4PPT9z41D0Pdyn1Q/X+bL0A/30k6gKhtlmqgsNqtalaMStx7en2Y2kiXPhRhKPFYvAtD2+M4XMy3JW0ibHQWLUpZVTeRNkzF3W/y7G4JSvSkz4oHGyYKajRyl4R0UxoKgwfzQYwPkzog= X-MS-TrafficTypeDiagnostic: MWHPR07MB3101: X-Microsoft-Exchange-Diagnostics: 1; MWHPR07MB3101; 20:OdpioKCmfn/agFyyfX1mofSlHZaX2Ff3pZMj1EH6A70Z+wl2pGvx3aaJtZL/uJJAkpBpG2CL1m79rCV2ruQZJ7M/u4vkJ7mna8W8Kpsbg00apWkvVdbiy2TG2BBoVUEl2WZr2x0lG9gQrMMPnzcrBkbynljulm4OcGI4AopKZ4fsJnQQpD9ndj5rgvgDPXnViMoBUZHysmSnA3QuF5T+tk69qM1R7oagD1USvQysH86Phw3eLuJ4p+xPj49f+//tXxM6/7Fbx+aJVZ1Ft00cvelRsMPOTmjJ5JI5RdGeqnsObaLrs/9g7xzzMFOKzHRXpcYyTgF88rO4C++2e4VcOwW4kNQgNtzhqKK3SutDjQful/jEpcoBJaSta188dYWlIBIvXO/PMMhDp3/SvG5y0eJnX2FFUyLuB5+9egFyKgpHwsfpx3EoN9Fr9/azPTWbY/rtO8S6RLSkLa3BuY1NTcU88Z9OKsrhYq8FWCjc7qct3phShGLjc71o/g9NECjBzr5yWv66GJiQZ9vaErsfLPKYHlttI+I72cL0RuBAVIPKuCsTwwy1okoERR82GIRayPzBTQL+alUhklj62GD6rWwaKcqgHsHjbG4kemqimtM=; 4:MOk9Koh/TM91ophzJPiFAcTBSTASdb4zuv2tvz2mjXHHHgpCoI3yPAy6mIl3WQtzBso1HqnYcdNAQ5Ti0aJUEcc9a9XfPhKrOfDV4Bzre1LEc47xaBlE9NW2LV7c3DXoj7w3ydWe9C3XYqxRCNJ2vSGwvYXxaOvB9C+BJCEoNTTQQ2zZWlHDlKDq347K5hAjzjoJZRJIrQkzOntmdFa0LAzsVus1j3D8JGSjFcZjdVKYUF4nSWUNwOTYq41foJwUFOzul2ldGw4GZKcEPGTxWD8XJGLC3+uLY2LONSsdGN0= X-Exchange-Antispam-Report-Test: UriScan:(185117386973197); X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(100000703101)(100105400095)(3002001)(93006095)(10201501046)(6041248)(20161123558100)(20161123564025)(20161123562025)(20161123560025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:MWHPR07MB3101; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:MWHPR07MB3101; X-Forefront-PRVS: 0457F11EAF X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6049001)(6009001)(376002)(346002)(24454002)(199003)(377454003)(189002)(81166006)(93886005)(117156002)(189998001)(966005)(64126003)(16576012)(83506001)(50466002)(50986999)(76176999)(33646002)(8656003)(54356999)(316002)(2906002)(106356001)(65956001)(66066001)(230700001)(6116002)(105586002)(23746002)(97736004)(36756003)(65806001)(3846002)(65826007)(31696002)(101416001)(110136005)(5660300001)(58126008)(42882006)(2950100002)(68736007)(7736002)(53936002)(81156014)(8676002)(25786009)(6666003)(229853002)(305945005)(90366009)(72206003)(77096006)(8936002)(6486002)(86152003)(16526018)(478600001)(6306002)(4326008)(31686004)(53376002)(6246003)(47776003); DIR:OUT; SFP:1101; SCL:1; SRVR:MWHPR07MB3101; H:[192.168.0.105]; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: cavium.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1; MWHPR07MB3101; 23:ty3YqwrvwJQThy2iJF2zVIrYh4/GLI63q79LU?= =?Windows-1252?Q?PDRgaUy6lhsOjwAG/t61WBQS++M24GxcLwAMkFRbbNdds5MnNFO5ITYN?= =?Windows-1252?Q?sf/U6p2Uc+UgicAKn5VhkeMqIc4ZeHeOb3pRNiu3FVt/LSn3ROzGQ73c?= =?Windows-1252?Q?p5J23G9fP1zLNQBOjRQ9Guj/xpyu0DCyNdwywwAuQD3w4KQkX+aAhc+G?= =?Windows-1252?Q?XnwwQFIMiZrQCh/8T6HIB67VlXulbVZl1o3cktGxEj2hblglggtlBfln?= =?Windows-1252?Q?DxzobHxGsCr5ak1UWR7ZVs9r95lEX+zZzYSjiLbMAc48v6ekLF7QxLHT?= =?Windows-1252?Q?wOGAwOcgpEQegAva45v5e9DYesh2J6wKx/fefIOTtGoZ4CmQZhKDmJQ7?= =?Windows-1252?Q?d9bLCW1lrfbMAxJLXfR7Oo6Om0DeJecpMsz2xJcgk3woGu/6B277dVM2?= =?Windows-1252?Q?WJ2Jals33ppDX80G41mUcx5BPDdoBwwtPwF6GRi19kpfpv0v8pftjrG2?= =?Windows-1252?Q?L77UHzUJxS56Talo/8+ph9mbz7uNZ9cjU6EyhmXAiHleBdGho2anZaI+?= =?Windows-1252?Q?RkSF7MzfHURIdR9KWIkCqza+UNakY8H3sdRfXqwsYdeC7xYudvWks8Cv?= =?Windows-1252?Q?pdh3FJXvGrjGTd4GvtT/RUmMfSMND/1j80vuFCmrTgm5oh545X3XdEAO?= =?Windows-1252?Q?U0EjbxOuIXQn3OLVFcC2s9k+TA/Ee3H38aMcnL3syw9XPy+8i65OY9gS?= =?Windows-1252?Q?Lq5tdrCXmgjg9vzpaBCFA0PFVrCQ1v3e/qnB6Ah0qozTVi7zGNsxcRKX?= =?Windows-1252?Q?s3PovBwAPkMjiPLOSIlRjiAHswIwApJou+mpd6/b6b18jWiKVKSdm/bh?= =?Windows-1252?Q?mv+hCbuHzqvNaTiLg2mENESjmaYP5HWmErfq+XulUblQXNmbUAXwMWjA?= =?Windows-1252?Q?fnEwuT/+5h2DMYlrtKwNecpVM1iGJG2/kJBUmIAER1nkl5q+BIYVTjQ9?= =?Windows-1252?Q?jl27fAvDmqGUi9mjTApe+ghb77gjfyyT0z4VRKJpHQeP0VhFKhmHJP2q?= =?Windows-1252?Q?UpwJR35NyD0devZdXu80zNWs3Cy+0SlAFnzFkBScvHhEhdGgw5NO8Npi?= =?Windows-1252?Q?ZPRiqQZ9W2cmJ4z2RlQiBXXYxHlTfOCAGl+Ks4tdXGFGZ7u773GMsrkj?= =?Windows-1252?Q?ahUFTKlERsmsXKJV5x3nsqjGaBqZ4b6b57TEeHQ08VWJOg8jw/MHPps0?= =?Windows-1252?Q?hM+pmiBqlQaczW3yYIxnhiUWAJxlqOdMiSk/SZ934OSnGn1nXV6N5IDz?= =?Windows-1252?Q?ypjDWzpkrFYkQS4ffNLJdn92NjyfdI/M75CdM6t6yPQtwSM+M0L+JYuw?= =?Windows-1252?Q?ct5BftL7iK0pnnkHbPc0jJtCJbu4r1/slc3/JMAHyE++Gh6bYLZVcLh7?= =?Windows-1252?Q?x5SCsYAwc/45FGUA4EVZ+jJWfTydleWo27K0agAZCE6F4jRsOa3ZKXN2?= =?Windows-1252?Q?r90nDK6pYMKzSWIUeb60MYyMstqhQQ+a/p3wuK3jhNYxWZ3PwNgAr3Sp?= =?Windows-1252?Q?vacxDUR7SAUfQ59zM6Z4AqII1EBknVUuG710b9btVJOIbm0HyzrfNeie?= =?Windows-1252?Q?TjruwFhWo6VLxI+6bNaYb8=3D?= X-Microsoft-Exchange-Diagnostics: 1; MWHPR07MB3101; 6:I51os8lMpljDShm7nHuNEgDXT7/HRgPdsDT2xbWBKnuA/rLv53xUfKx7p56Wm8r+jslP/0YweU4vCxmya2FGf7HeGTVgxQ3AAbghTeBvtDTvvDgGDsaJ7reU0y7kQqXBpvhzXHS8mlFNUcfAa7uRfGbZuGoWBb0/5c5t9Hp7E7C0ti8/WSoPkvb+Jp2QyZjhhZfsBrnh1vu3oCFi680+9FzMntGWn605LojPUsRkgTNK2uwon5hue8d2oiN7wYkd39jIH9ior2rMktnaYU9J/HgbmG0EiMj1JJpv+11CySx917ib7Rib1Klk5I7WhoVPCZ2VKchm+CnWGGbASsyznQ==; 5:1j2h0z59J1Jz16wiW6r7a6Zsa8e7WFb6AyizDOuaHMLNryrJ0mrCgULxyIYz6dersT4iwH9h/OVozbJRLi1e8qnjNqCeJuatO5p6Zpp1MniEVp6qxOEM8loea0KyXb0cPLI6zrmHh4yqI7r4s2IkFw==; 24:j6/vn5LgFBMnoyWIsjlie71ZiYOsF8yLyKkyjGPlSD3Gh1mpEce9J0/1fgDj/+c/v2EcgfbDvDuCpTEON3Un0aPoRStmh+JbH5gfP8pmvTU=; 7:ZfSP/FfdYBTnFqVXReriW/bi3drHE4LhuwvI42+gQhKei8H/5iMluJCothk2K6sKQL+3IEIth8jlTXherIt+Vt5dyG2CWoCjHrpjWfVfNeVszHoJ4IcdKPSWtDDZjVniH/2RW7gmxwqOGidhciScQWNf8i0zyjM46z5g1mQUW3OVBSsu++adDUiuy8RB+4p6R6UncPK+Z4JySxXguaqh6Hm5cOSxuIc9DSePqmOaBGI= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Oct 2017 05:23:40.9367 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR07MB3101 Subject: Re: [dpdk-dev] [PATCH] bus/dpaa: fix memory allocation during bus scan 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, 11 Oct 2017 05:23:45 -0000 On Wednesday 11 October 2017 10:47 AM, Shreyansh Jain wrote: > On Tuesday 10 October 2017 10:25 PM, santosh wrote: >> >> On Tuesday 10 October 2017 09:47 PM, Thomas Monjalon wrote: >>> 10/10/2017 16:05, santosh: >>>> Hi Thomas, >>>> >>>> On Tuesday 10 October 2017 01:09 PM, Thomas Monjalon wrote: >>>>> 10/10/2017 09:01, Shreyansh Jain: >>>>>> Fixes: 5b22cf744689 ("bus/dpaa: introducing FMan configurations") >>>>>> Fixes: 37f9b54bd3cf ("net/dpaa: support Tx and Rx queue setup") >>>>> These lines should appear after the explanation. >>>>> >>>>>> Cc: shreyansh.jain@nxp.com >>>>>> >>>>>> With the IOVA auto detection changes, bus scan is performed before >>>>>> memory initialization. DPAA bus scan must not use rte_malloc in >>>>>> its path. >>>>> If the scan has been broken by IOVA detection, you should reference >>>>> IOVA in Fixes line, not DPAA. >>>>> >>>> hmm.. IOVA not breaking scanning!, Refer this [1]. >>> It is breaking. A break is a behaviour or interface change. >>> When moving init order, you break behaviour. >>> I don't say it is bad. >>> I say only it is the primary cause of this change. >> >> disagree!. Why so: Legacy PCI/bus scan implementation don't >> use rte_ lib as they don;t need to.. Refer [1] for detailing. >> However, dpaa is and that we agree to align with legacy. >> So its a open question : Who fixes who? > > Just because PCI/bus-scan didn't use rte_malloc didn't mean that no one else can use it. For DPAA, the case was different and ideally I shouldn't have used That means that Scan should do scan only. if not then programmer creating a dependency like it was in past. > rte_malloc. [1] was a discussion based on FSLMC and that was incorrect implementation. > yes But thread did address on _not_ keeping any rte_ memory dependency at scan time. > But, that is not a general case. > > If we go by pre-IOVA patch era: > > -> Subsystem A initialized > -> Subsystem B initialized > -> bus scan > `-> implementation free to use all initialized subsystem > > post-IOVA era > > > -> Subsystem A initialized > -> bus scan > `-> implementation free to use all initialized subsystem > -> Subsystem B initialized > And that the problem with Pre-era that some platform implementation created a dependency on scan which wasn't necessary and that was blocking new infra. > Essentially, it means IOVA made a change in the expected behavior of the implementation. I disagree. IOVA asked to remove those dependency for those platform., If PCI/BUS works w/o dependency then why not fslmc that same we discussed in thread. You have created a programming dependency in your platform code at scan time, which after discussion agreed and now this patch is removing that dependency. Ideally, Like for other Patches hemant did: You should have blocked IOVA series and let this patch and other platform dependency patch merged before IOVA.. but that didn't happen perhaps lack of co-ordination. > For case of DPAA, as that was still in development, it certainly can be said that it should have been fixed within dev cycle itself. That was the prime reason I used 'Fixes' to my own patches in v1 of this patch. > > But that said, I agree with what Thomas is saying. Fixes is only indicative of which patched changed the status-quo. And it helps maintainers know dependency tree. In this case, DPAA patches came *before* IOVA was added and hence the mistake in committed version came out *after* IOVA patches. > You should have blocked IOVA in that case, as said above like Hemant did for one of his patch. Anyways, I'm not objecting on Fixes: tags valid or not. Yes, from git point of view it is correct But from design point of view - I disagree. >> >>> The Fixes: line is also a help when backporting patches. >>> This patch needs to be backported only if IOVA patch is also backported. >> >> IMO, would prefer backport: rather fixes: tag in above case, more verbose I guess. >> >>>> We(me/hemant) has discussed about same on thread[1] and agreed to >>>> do respective changes and remove rte_ memory dependency from code base >>>> at scan time.. >>>> >>>> Thanks. >>>> >>>> [1] http://dpdk.org/dev/patchwork/patch/26764/ >>> You already discussed about this issue, fine. >>> >>> Santosh, as you insist to talk again about it, one more comment: >>> >>> It is very good to have discussions on the mailing list. >> >> Thanks, That makes me think that I didn't break, indeed did what was needed in agnostic way. > > Again, IOVA patch changes status quo, but that was a known fact and it was missed by DPAA patches. Now, for maintenance reasons, "Fixes" should actually be IOVA patch. I am not sure why you disagree with that. > Read above reason for disagreement. >> >>> It would be perfect if all these informations were explicitly given >>> in the commit messages. >>> For instance, saying that the scan cannot use rte_malloc anymore is >>> a valuable tip for other developpers. >> >> Agree, but scan wasn;t using for PCI/bus case.. so one can;t be sure whether to >> mention or not.. >>