On 10-Jun-20 5:39 PM, Dmitry Kozlyuk wrote:

On Wed, 10 Jun 2020 16:48:58 +0100
"Burakov, Anatoly" [off-list ref] wrote:

quoted

On 10-Jun-20 3:31 PM, Dmitry Kozlyuk wrote:

quoted

On Wed, 10 Jun 2020 11:26:22 +0100
"Burakov, Anatoly" [off-list ref] wrote:

[snip]

quoted

quoted

quoted

quoted

+	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