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 CB094A051F;
Wed, 10 Jun 2020 18:39:50 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 2D5231BFB3;
Wed, 10 Jun 2020 18:39:50 +0200 (CEST)
Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com
[209.85.208.195]) by dpdk.org (Postfix) with ESMTP id 3120A1BFA7
for <dev@dpdk.org>; Wed, 10 Jun 2020 18:39:49 +0200 (CEST)
Received: by mail-lj1-f195.google.com with SMTP id e4so3328039ljn.4
for <dev@dpdk.org>; Wed, 10 Jun 2020 09:39:49 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
h=date:from:to:cc:subject:message-id:in-reply-to:references
:mime-version:content-transfer-encoding;
bh=Ml1d2ap0lT1Xy+ZvRlUVSskELldU9OjdWK5pJZAtxoY=;
b=DBU5J2hMCkIB/gO3fcNRzN6MmITU3H3pnPGTyiEGwj3f89lhhN1OW1hlnRydXdlEdz
mq8QRLzt8pi6ZKZqY7WjTATV1i8suuIXLfTTrvBDsFIWQjYR30LII7o0kuhb0Q2TEtr8
oYYb0j7JqF1/xFFX/lR2n4vVXzvCUl7bEKzMIvlidhFd03143OBc9tf2qT0fneaw5YUH
OQaSw8D5JUyYccD1PdMQI4YUNmuA6MaTJGNtBuFNMu7/nLZKdtv2e/swSuWyggblvDZR
frkXDulsxvTCb2DyFM/+sUwY761vL4nhh4BLpIscVMLWbJLC8oBLZKgc1/Zii7TdDQkX
lxAA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to
:references:mime-version:content-transfer-encoding;
bh=Ml1d2ap0lT1Xy+ZvRlUVSskELldU9OjdWK5pJZAtxoY=;
b=MhCublqdunPfFGVZB39M38dUUWcrhkRIsCOeOHyZp+3By3bemGQienl/u99kZTXry4
Wzc4hW43K3AHAvrsULKyx6GdXOLGUMUTVFfoKmzjM+6Kn5xid7nn4Zhq1u7//H8ZTOtn
y0m86eUv++t6fi4T5NqTb7tkEUU2I22HpGeFfKm94HTC8mTNSsud98xiA84IAE0wKbWm
vRI5IKQTd/HTLg7TDpek1vcZiq/B7bplWlIoXAX1q+cIZSZbQTNtA9QikshW/QIDC8jw
6mM4F7A0jIS0jtQsIoO8yi4kdjMaTZjyiUNciOa3eAj9EjqfSAFxtal48/Rcb0Hl5v7J
ZDjg==
X-Gm-Message-State: AOAM531VCIuxCkB+vSTMMaUL0tKrfWDP8o8jOu21/tbOqASUFEg+dDJr
/c4Y1bGypCSpw0OGoRxcCWY=
X-Google-Smtp-Source: ABdhPJwWHbiDWOoVjFCX9FRPHzFnycvGsjG+fjcjRSwLubB/rPdC09pB4P40iveyIvjzePUZwhrUUg==
X-Received: by 2002:a2e:390a:: with SMTP id g10mr2088199lja.373.1591807188560;
Wed, 10 Jun 2020 09:39:48 -0700 (PDT)
Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru.
[37.110.65.23])
by smtp.gmail.com with ESMTPSA id q8sm52455lfo.13.2020.06.10.09.39.47
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Wed, 10 Jun 2020 09:39:47 -0700 (PDT)
Date: Wed, 10 Jun 2020 19:39:46 +0300
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.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>
Message-ID: <20200610193946.2c1d704a@sovereign>
In-Reply-To: <201ca89a-ae94-0bc1-184f-ac8f41b38b57@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>
X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
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 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.
--
Dmitry Kozlyuk