From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; Wed, 10 Jun 2020 18:39:49 +0200 (CEST) Received: by mail-lj1-f195.google.com with SMTP id e4so3328039ljn.4 for ; 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 To: "Burakov, Anatoly" Cc: dev@dpdk.org, Dmitry Malloy , Narcisa Ana Maria Vasile , Fady Bader , Tal Shnaiderman , Bruce Richardson 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> <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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, 10 Jun 2020 16:48:58 +0100 "Burakov, Anatoly" wrote: > On 10-Jun-20 3:31 PM, Dmitry Kozlyuk wrote: > > On Wed, 10 Jun 2020 11:26:22 +0100 > > "Burakov, Anatoly" 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