From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Rybchenko <arybchenko@solarflare.com>
Subject: Re: [RFC v2 01/17] mempool: fix phys contig check if
 populate default skipped
Date: Thu, 1 Feb 2018 09:54:35 +0300
Message-ID: <b10db3a0-0b41-af79-279f-1a967823fe8d@solarflare.com>
References: <1511539591-20966-1-git-send-email-arybchenko@solarflare.com>
 <1516713372-10572-1-git-send-email-arybchenko@solarflare.com>
 <1516713372-10572-2-git-send-email-arybchenko@solarflare.com>
 <20180131164504.cnfgfwo2x3ftxnaj@platinum>
 <d68cb16b-bfe9-083b-a83e-e40b8eb5aaa0@caviumnetworks.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Cc: <dev@dpdk.org>, <stable@dpdk.org>, <jerin.jacob@caviumnetworks.com>
To: santosh <santosh.shukla@caviumnetworks.com>, Olivier Matz
 <olivier.matz@6wind.com>
Return-path: <dev-bounces@dpdk.org>
In-Reply-To: <d68cb16b-bfe9-083b-a83e-e40b8eb5aaa0@caviumnetworks.com>
Content-Language: en-GB
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 02/01/2018 08:05 AM, santosh wrote:
> On Wednesday 31 January 2018 10:15 PM, Olivier Matz wrote:
>> On Tue, Jan 23, 2018 at 01:15:56PM +0000, Andrew Rybchenko wrote:
>>> There is not specified dependency between rte_mempool_populate_default()
>>> and rte_mempool_populate_iova(). So, the second should not rely on the
>>> fact that the first adds capability flags to the mempool flags.
>>>
>>> Fixes: 65cf769f5e6a ("mempool: detect physical contiguous objects")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> Looks good to me. I agree it's strange that the mp->flags are
>> updated with capabilities only in rte_mempool_populate_default().
>> I see that this behavior is removed later in the patchset since the
>> get_capa() is removed!
>>
>> However maybe this single patch could go in 18.02.
>> +Santosh +Jerin since it's mostly about Octeon.
> rte_mempool_xmem_size should return correct size if MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag
> is set in 'mp->flags'. Thats why _ops_get_capabilities() called in _populate_default() but not
> at _populate_iova().
> I think, this 'alone' patch may break octeontx mempool.

The patch does not touch rte_mempool_populate_default().
_ops_get_capabilities() is still called there before
rte_mempool_xmem_size(). The theoretical problem which
the patch tries to fix is the case when
rte_mempool_populate_default() is not called at all. I.e. application
calls _ops_get_capabilities() to get flags, then, together with
mp->flags, calls rte_mempool_xmem_size() directly, allocates
calculated amount of memory and calls _populate_iova().

Since later patches of the series reconsider memory size
calculation etc, it is up to you if it makes sense to apply it
in 18.02 as a fix.