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

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.

-- 
Thanks,
Anatoly