From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 4C855A00C5;
Thu, 11 Jun 2020 10:59:35 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 24E281B94F;
Thu, 11 Jun 2020 10:59:35 +0200 (CEST)
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
by dpdk.org (Postfix) with ESMTP id 425D65F69
for <dev@dpdk.org>; Thu, 11 Jun 2020 10:59:34 +0200 (CEST)
IronPort-SDR: 3v7UHguHxJFmXfdwbm6kH8z8gTF6XKYzvzn3CHDVTA+GB2PK3+i6yQDGXD52zLkfXDrrD+snnD
R+95Z4R6We1A==
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga008.jf.intel.com ([10.7.209.65])
by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
11 Jun 2020 01:59:33 -0700
IronPort-SDR: wFUqKn9mqZpkoTHSpDHZdcrK9Jj8hfL5mapNIn31ZJHP1WkEFr8jA1NMExOPCpldC2sVjSQzhK
ltFEG+D3w8bw==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.73,499,1583222400"; d="scan'208";a="306867160"
Received: from strampit-mobl.ger.corp.intel.com (HELO [10.252.45.191])
([10.252.45.191])
by orsmga008.jf.intel.com with ESMTP; 11 Jun 2020 01:59:31 -0700
To: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Cc: dev@dpdk.org, Dmitry Malloy <dmitrym@microsoft.com>,
Narcisa Ana Maria Vasile <Narcisa.Vasile@microsoft.com>,
Fady Bader <fady@mellanox.com>, Tal Shnaiderman <talshn@mellanox.com>,
Bruce Richardson <bruce.richardson@intel.com>
References: <20200525003720.6410-1-dmitry.kozliuk@gmail.com>
<20200602230329.17838-1-dmitry.kozliuk@gmail.com>
<20200602230329.17838-5-dmitry.kozliuk@gmail.com>
<255a3887-b187-e3b2-cf06-a0a67942e788@intel.com>
<20200609171733.7e3d9601@sovereign>
<a3eaab78-410b-c957-b1dd-84b5b219dc64@intel.com>
<20200610173151.27f1a1aa@sovereign>
<201ca89a-ae94-0bc1-184f-ac8f41b38b57@intel.com>
<20200610193946.2c1d704a@sovereign>
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Message-ID: <698e0cac-3fe0-d54b-8a43-d0dbf99c56d2@intel.com>
Date: Thu, 11 Jun 2020 09:59:30 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
Thunderbird/68.8.1
MIME-Version: 1.0
In-Reply-To: <20200610193946.2c1d704a@sovereign>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v6 04/11] eal/mem: extract common code for
memseg list initialization
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>
On 10-Jun-20 5:39 PM, Dmitry Kozlyuk wrote:
> On Wed, 10 Jun 2020 16:48:58 +0100
> "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
>
>> On 10-Jun-20 3:31 PM, Dmitry Kozlyuk wrote:
>>> On Wed, 10 Jun 2020 11:26:22 +0100
>>> "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
>>>
>>> [snip]
>>>>>>> + addr = eal_get_virtual_area(
>>>>>>> + msl->base_va, &mem_sz, page_sz, 0, reserve_flags);
>>>>>>> + if (addr == NULL) {
>>>>>>> +#ifndef RTE_EXEC_ENV_WINDOWS
>>>>>>> + /* The hint would be misleading on Windows, but this function
>>>>>>> + * is called from many places, including common code,
>>>>>>> + * so don't duplicate the message.
>>>>>>> + */
>>>>>>> + if (rte_errno == EADDRNOTAVAIL)
>>>>>>> + RTE_LOG(ERR, EAL, "Cannot reserve %llu bytes at [%p] - "
>>>>>>> + "please use '--" OPT_BASE_VIRTADDR "' option\n",
>>>>>>> + (unsigned long long)mem_sz, msl->base_va);
>>>>>>> + else
>>>>>>> + RTE_LOG(ERR, EAL, "Cannot reserve memory\n");
>>>>>>> +#endif
>>>>>>
>>>>>> You're left without any error messages on Windows. How about:
>>>>>>
>>>>>> const char *err_str = "Cannot reserve memory\n";
>>>>>> #ifndef RTE_EXEC_ENV_WINDOWS
>>>>>> if (rte_errno == EADDRNOTAVAIL)
>>>>>> err_str = ...
>>>>>> #endif
>>>>>> RTE_LOG(ERR, EAL, err_str);
>>>>>>
>>>>>> or something like that?
>>>>>>
>>>>>
>>>>> How about removing generic error message here completely and printing more
>>>>> specific messages at call sites? In fact, almost all of them already do this.
>>>>> It would be more helpful in tracking down errors.
>>>>>
>>>>
>>>> Agreed, let's do that :) We do pass up the rte_errno, correct? So, we
>>>> should be able to do that.
>>>
>>> Actually, callers don't need rte_errno, because we only have to distinguish
>>> EADDRNOTAVAIL here, and eal_get_virtual_area() already prints precise
>>> diagnostics at WARNING and ERR level. rte_errno is preserved, however.
>>>
>>
>> Not sure i agree, we still need the "--base-virtaddr" hint, and we can
>> only do that from the caller (without #ifdef-ery here), so we do need
>> rte_errno for that.
>
> Maybe we're talking about different things. The "--base-virtaddr" hint is
> printed from eal_memseg_list_alloc() on Unices for EADDRNOTAVAIL.
> This is handy to avoid duplicating the hint and to provide context, so let's
> keep it despite #ifndef.
>
> Otherwise, a generic error is printed from the same function (mistakenly not
> on Windows in v6). This generic error adds nothing to eal_get_virtual_area()
> logs and also doesn't help to known which exact eal_memseg_list_alloc()
> failed. If instead callers printed their own messages, it would be clear
> which call failed and in which context. Generic error can than be removed,
> eal_memseg_list_alloc() code simplified. Callers can inspect rte_errno if they
> ever need it, but really they don't, because hint is printed by
> eal_memseg_list_alloc() and eal_get_virtual_area() prints even more precise
> logs. This is what I did in v8.
>
Right, OK :)
--
Thanks,
Anatoly