From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8C842A0487 for ; Thu, 4 Jul 2019 10:09:26 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7B0D31B942; Thu, 4 Jul 2019 10:09:25 +0200 (CEST) Received: from mail-ua1-f65.google.com (mail-ua1-f65.google.com [209.85.222.65]) by dpdk.org (Postfix) with ESMTP id 4F76B58C4 for ; Thu, 4 Jul 2019 10:09:23 +0200 (CEST) Received: by mail-ua1-f65.google.com with SMTP id o2so843690uae.10 for ; Thu, 04 Jul 2019 01:09:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=m15tln+xSLFAiJwV8sEsy3cEStEvquKvPWdIP8LBDgU=; b=o1aSIsUhK2z3EYqN8k5N+xSdbZOvPPWxMUKOeXr5OCIx5pLCT39/Eiv9Oa3CDnslB1 HfboZOjDP4aJJ1/ZEA3K2NLbzcVC3cJC/JjRIXAeFommybxf84n1HknPb66VRxUnXrnM HIqhSQ0yCOVnFXrq4opoFbMKc9GfA1KzwEQhP2eY1hl9ULhC9Zo0jcD5iO3y2OLBIaAl l23u/z57rAt+2+pHdiY17pQ7+6SWwVDDGlW/D/4dsYfRVFUEO6BVm8jbgP7LnV1Qpk1q TX8bu3B/lWuUrql4GewUwL0XRNFbpco8e3QYOO+bssbaNUa/rJX+WPdgZzJRIHxuuF42 5hNQ== X-Gm-Message-State: APjAAAXhbUksMvWK+63tI7G8UtLooH7Ps4z5j/+zahPr6LV/qFwIYfN5 qvcbApPlUaRm1wW8rZKmqAS2FwEhZ2xqhdJgPd9n9w== X-Google-Smtp-Source: APXvYqzHXvXLOTCwgZhX5COXJEsNXBNfLaALrqvwP3zI0Bf7iifNSvstNyHPrbUysPshpr8p5NE9ODEjIX3TM8IzsBc= X-Received: by 2002:ab0:45e3:: with SMTP id u90mr3931282uau.126.1562227762622; Thu, 04 Jul 2019 01:09:22 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: David Marchand Date: Thu, 4 Jul 2019 10:09:11 +0200 Message-ID: To: Anatoly Burakov Cc: dev , Thomas Monjalon , Stephen Hemminger Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v3 00/14] Make shared memory config non-public 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Jul 3, 2019 at 11:38 AM David Marchand wrote: > Hello Anatoly, > > On Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov > wrote: > >> This patchset removes the shared memory config from public >> API, and replaces all usages of said config with new API >> calls. >> >> A lot of the patchset is a search-and-replace job and should >> be pretty easy to review. The rest are pretty trivial EAL >> changes. >> >> This patchset depends on FreeBSD fixes patchset: >> >> http://patches.dpdk.org/project/dpdk/list/?series=5196 >> >> v3: >> - Rebase on top of latest master >> >> v2: >> - Collapsed all changes into fewer patches >> - Addressed review comments >> - Created a new file to store the code >> - Changed namespace to "rte_mcfg_" >> - Added some unification around config init >> - Removed "packed" attribute from mem config >> - Removed unnecessary inlining >> - Added a check to explicitly forbid running multiprocess >> applications that differ in their DPDK versions >> > > > For the parts I already had a look at, I still think the changes are in > too many patches. > A lot of this is just search/replace we can have it with the patch that > introduces it. > - patch 1, 2 and 3 could be squashed as a single one (plus removing the > unused macro from patch 8) > - idem with patch 4 and 5 > - idem with patch 6 and 7 (plus removing the unused macro from patch 8) > > Overall, I am ok with the changes, once the patch 13 is fixed. You can add my ack on the n+1 patchset. I just want to state two approaches to merge these changes: - the first one would be to split this series in two - take the first part of this series now, but mark the new API "experimental" - postpone the merge to 19.11 of the second part, which starts at the hiding rte_mem_config patch - the second one is taking these changes in one go The second one is the quicker and the more straightforward but it leaves the risk of having missed something and we must break the ABI again in 19.11. The risk is quite low, given the changes. Thomas, comments? -- David Marchand