From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D4819A0C43; Thu, 23 Sep 2021 10:12:20 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7209741249; Thu, 23 Sep 2021 10:12:12 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mails.dpdk.org (Postfix) with ESMTP id C5B46410DC for ; Thu, 23 Sep 2021 10:12:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632384728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mh74gJsvgHrAN3YgyJpXArkdskCZ1WZAhG5ypBet7VM=; b=jCIwrYQLDxloCERdQjaZIIiee2iI5cV/DkAcA4l2EU0NVGPtt7YgQYRoIdQh+ubW26/l1P 6mlOvQxQ3CK19XRfH3aKglExbInWzmx8E2r2vpVpn/rbzOcXbD+nrGo5e1zuiVOJd6rKnm +32cJdfSTTKgPDVO3am24Uvp/kZHamk= Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-313-QpES2WklMqeuGBi_cQwnVg-1; Thu, 23 Sep 2021 04:12:06 -0400 X-MC-Unique: QpES2WklMqeuGBi_cQwnVg-1 Received: by mail-lf1-f70.google.com with SMTP id a28-20020a056512021c00b003f5883dcd4bso5425181lfo.1 for ; Thu, 23 Sep 2021 01:12:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mh74gJsvgHrAN3YgyJpXArkdskCZ1WZAhG5ypBet7VM=; b=6mrUUPtq8+d67eUeLsEM6AY9rVPuijUgXlsOcn+i0GqsL0USbNzVUgmM5A0O7+LL2P rxL8trJ24pbelhTgwfctDNGaV59wwtH17x3SaDDUHSzwice+BQVd6yWZMTkT3wMRJMpD Sww+/3TNlb6sLfR9oDYrDSJwWnW4T7DMSg5Q0G1h+H5W/S/mr0kcyQM00Ke/JXnV5ZsS jAgy23h7L0xb2f11EVx0njS144RJW8wlfCgja0LGdQJddillUR6M/ijFugmZXR71N1nX eRKG4f0qS7SAPbzI382I93fmrGAnDLvtFmsJNZcT56bDUGSOMVZdxUCvSAxEyO7rQhYq gcOA== X-Gm-Message-State: AOAM5315uZaknFbiXG6N8EagqLXXXJWtogNdeS5On0at4Ssjv4QB1Xa6 ZJoQaa6BDRNrnu2PWMHZtZ7nRoMeMdEg1dYGzdN+78nuU82YSMI/fgGot/UtSuxTOQxzI1yQ9qT ZzqzGNbohDXmxkzBdFZA= X-Received: by 2002:a05:6512:a8e:: with SMTP id m14mr2996949lfu.575.1632384725331; Thu, 23 Sep 2021 01:12:05 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxT8eAYn94zTbguozib7kEJg7gBFehWF47iVx12C2gwtROeD/Jm7o8nsHznLYUVnhk8a8Ha0Tkgy6XiuoswNjM= X-Received: by 2002:a05:6512:a8e:: with SMTP id m14mr2996924lfu.575.1632384725015; Thu, 23 Sep 2021 01:12:05 -0700 (PDT) MIME-Version: 1.0 References: <20210921115015.36442-1-david.hunt@intel.com> <20210922122920.34759-1-david.hunt@intel.com> In-Reply-To: <20210922122920.34759-1-david.hunt@intel.com> From: David Marchand Date: Thu, 23 Sep 2021 10:11:53 +0200 Message-ID: To: David Hunt Cc: dev , Bruce Richardson , Thomas Monjalon Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v4 1/2] eal: add additional info if core list too long X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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, Sep 22, 2021 at 2:29 PM David Hunt wrote: > > If the user requests to use an lcore above 128 using -l, > the eal will exit with "EAL: invalid core list syntax" and > very little else useful information. > > This patch adds some extra information suggesting to use --lcores > so that physical cores above RTE_MAX_LCORE (default 128) can be > used. This is achieved by using the --lcores option by mapping > the logical cores in the application to physical cores. > > For example, if "-l 12-16,130,132" is used, we see the following > additional output on the command line: > > EAL: lcore 132 >= RTE_MAX_LCORE (128) > EAL: lcore 133 >= RTE_MAX_LCORE (128) > EAL: to use high physical core ids , please use --lcores to map > them to lcore ids below RTE_MAX_LCORE, > EAL: e.g. --lcores 0@12,1@13,2@14,3@15,4@16,5@132,6@133 > > Signed-off-by: David Hunt > Acked-by: Bruce Richardson This series is there to help users, it should not break existing working configurations. I mentionned the "-l 0-3,0" case before. This syntax is debatable, but it worked before (see comment below) and this patch now refuses it. > --- > changes in v2 > * Rather than increasing the default max lcores (as in v1), > it was agreed to do this instead (switch to --lcores). > * As the other patches in the v1 of the set are no longer related > to this change, I'll submit as a separate patch set. > changes in v3 > * separated out some of the corelist cheking into separate function > * added extra messages for the different failure conditions. > * changed allocation of the core strings from static to dynamic > * now prints out a message for each core above RTE_MAX_LCORE > changes in v4 > * tweaked log messages to be a bit clearer about mapping lcores > to physical cores. > * improved indenting of log messages. > * fixed bug in overrunning end of lcore array > * switched from strlcpy to strdup because of a clang error > --- > --- > lib/eal/common/eal_common_options.c | 100 ++++++++++++++++++++++++---- > 1 file changed, 87 insertions(+), 13 deletions(-) > > diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c > index eaef57312f..de1717946f 100644 > --- a/lib/eal/common/eal_common_options.c > +++ b/lib/eal/common/eal_common_options.c > @@ -703,6 +703,47 @@ update_lcore_config(int *cores) > return ret; > } > > +static int > +check_core_list(int *lcores, unsigned int count) > +{ > + unsigned int i, j; One index variable is enough. > + char *lcorestr; > + int len = 0; > + bool overflow = false; > + > + for (j = 0; j < count; j++) { > + if (lcores[j] >= RTE_MAX_LCORE) { > + RTE_LOG(ERR, EAL, "lcore %d >= RTE_MAX_LCORE (%d)\n", > + lcores[j], RTE_MAX_LCORE); > + overflow = true; > + } > + } if (!overflow) return 0; > + if (overflow) { > + /* > + * If we've encountered a core that's greater than > + * RTE_MAX_LCORE, suggest using --lcores option to > + * map lcores onto physical cores greater than > + * RTE_MAX_LCORE, then return. > + */ > + lcorestr = calloc(1, RTE_MAX_LCORE * 10); Ugly but works as long as RTE_MAX_LCORE < 10k. I hope static analysis tools won't complain with the snprintf below where there is no check on return value. > + if (lcorestr == NULL) { > + RTE_LOG(ERR, EAL, "Unable to allocate lcore string\n"); > + return -ENOMEM; > + } > + for (i = 0; i < count; i++) > + len = len + snprintf(&lcorestr[len], > + RTE_MAX_LCORE * 10 - len, > + "%d@%d,", i, lcores[i]); > + if (len > 0) > + lcorestr[len-1] = 0; len - 1 > + RTE_LOG(ERR, EAL, "to use high physical core ids , please use --lcores to map them to lcore ids below RTE_MAX_LCORE,\n"); > + RTE_LOG(ERR, EAL, " e.g. --lcores %s\n", lcorestr); > + free(lcorestr); > + return -1; > + } > + return 0; > +} > + > static int > eal_parse_coremask(const char *coremask, int *cores) > { > @@ -833,54 +874,87 @@ eal_parse_service_corelist(const char *corelist) > static int > eal_parse_corelist(const char *corelist, int *cores) > { > - unsigned count = 0; > + unsigned int count = 0, k; No need for k, count is enough. > char *end = NULL; > int min, max; > int idx; > + int lcores[RTE_MAX_LCORE]; > + char *corelist_copy; corelist_copy is unused. > > for (idx = 0; idx < RTE_MAX_LCORE; idx++) > cores[idx] = -1; > > + corelist_copy = strdup(corelist); > + if (corelist_copy == NULL) { > + RTE_LOG(ERR, EAL, "Unable to duplicate corelist\n"); > + return -ENOMEM; > + } > + > /* Remove all blank characters ahead */ > while (isblank(*corelist)) > corelist++; > > /* Get list of cores */ > - min = RTE_MAX_LCORE; > + min = -1; > do { > while (isblank(*corelist)) > corelist++; > if (*corelist == '\0') > - return -1; > + goto err; > errno = 0; > idx = strtol(corelist, &end, 10); > if (errno || end == NULL) > - return -1; > - if (idx < 0 || idx >= RTE_MAX_LCORE) > - return -1; > + goto err; > + if (idx < 0) > + goto err; > while (isblank(*end)) > end++; > if (*end == '-') { > min = idx; > } else if ((*end == ',') || (*end == '\0')) { > max = idx; > - if (min == RTE_MAX_LCORE) > + if (min == -1) > min = idx; > for (idx = min; idx <= max; idx++) { > - if (cores[idx] == -1) { > - cores[idx] = count; > - count++; Here, in the original code, "duplicates" were accepted/ignored. > + if (count < RTE_MAX_LCORE) > + lcores[count++] = idx; > + else { > + RTE_LOG(ERR, EAL, "Too many lcores provided. Cannot exceed %d\n", > + RTE_MAX_LCORE); > + goto err; > } > } > - min = RTE_MAX_LCORE; > + min = -1; > } else > - return -1; > + goto err; > corelist = end + 1; > } while (*end != '\0'); > > if (count == 0) > - return -1; > + goto err; > + > + if (check_core_list(lcores, count)) > + goto err; > + > + /* > + * Now that we've gto a list of cores no longer than typo. > + * RTE_MAX_LCORE, and no lcore in that list is greater > + * than RTE_MAX_LCORE, populate the cores > + * array and return. > + */ > + > + for (k = 0; k < count; k++) > + cores[lcores[k]] = k; do { count--; cores[lcores[count]] = count; while (count != 0); cores[count] > + > + if (corelist_copy) > + free(corelist_copy); > + > return 0; > +err: > + if (corelist_copy) > + free(corelist_copy); > + > + return -1; > } > > /* Changes the lcore id of the main thread */ > -- > 2.17.1 > -- David Marchand