From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 2B46B45EBC
for <public@inbox.dpdk.org>; Mon, 16 Dec 2024 11:03:07 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 217CC40144;
Mon, 16 Dec 2024 11:03:07 +0100 (CET)
Received: from us-smtp-delivery-124.mimecast.com
(us-smtp-delivery-124.mimecast.com [170.10.129.124])
by mails.dpdk.org (Postfix) with ESMTP id DBAA840144
for <stable@dpdk.org>; Mon, 16 Dec 2024 11:03:05 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
s=mimecast20190719; t=1734343385;
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:
content-transfer-encoding:content-transfer-encoding:
in-reply-to:in-reply-to:references:references;
bh=3s34NFGHjByUoWMyLU/JoYIXEroF0HRiE/e/USBWMf0=;
b=Qakf0Q6i5rpJw0bHZ/QjSC70J5GBKn5/22MeUYsP3lH25nSYS0ZuxTw/NpFsdfzjh85tAf
Eq0Z5bop8tO1mUKIuPZIPB+1wb6q2Y7qZKTJvks5g01gPlaBauGgnfwPJ5WPBtDV9OnSqE
9iD9M0CyoiMjrPj3vMA0+oKW0jy5C1w=
Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com
[209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS
(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id
us-mta-447-dptexSv3P3CfFwW3MXVdaw-1; Mon, 16 Dec 2024 05:03:04 -0500
X-MC-Unique: dptexSv3P3CfFwW3MXVdaw-1
X-Mimecast-MFC-AGG-ID: dptexSv3P3CfFwW3MXVdaw
Received: by mail-lf1-f70.google.com with SMTP id
2adb3069b0e04-53e396834daso323608e87.2
for <stable@dpdk.org>; Mon, 16 Dec 2024 02:03:04 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1734343383; x=1734948183;
h=content-transfer-encoding:cc:to:subject:message-id:date:from
:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc
:subject:date:message-id:reply-to;
bh=3s34NFGHjByUoWMyLU/JoYIXEroF0HRiE/e/USBWMf0=;
b=UQk9FYzCCO4aMYER7Zhjsi+wAkteK4BAacCV6UGo5Q+uOpiXe9uNHAbj4ZIo24GExF
aoM+4PODQ7zIcAJs9BbrpgdZqcQ/r9O00DrGG8LzL3eOvWiJy5J0YSoa66EKSokZ1b3f
uFy6AXeVDiIJY98gdfOuUy+ZZKLiwvWMskIjd6x/Rgknwg1ww4VUGDyPMAmGufZUcWlj
lbxxjpuBQu2y/9Z0M4Zuf7ys+Hd1pBRlgEamc5dzXDmsOLa8yY+yU09lrdnqQG0aAlHR
DiA+5jbXJwbSDC6gPqOMQVp8z7q/y+ihpCwfJMEHZNAYMmo/5D2UjTinaDGEPrKTR0/O
eosg==
X-Forwarded-Encrypted: i=1;
AJvYcCWqCJFXrsGCv5g5K8JblORPW1KYhRrU9f1cU/1V1g5/SLX5W0OnbOPEGDVbBGTzvHC+mylKhNA=@dpdk.org
X-Gm-Message-State: AOJu0YwSSCcYLnRRX6zJlUNszxZ0oUDH73irvCW1M/xqNinyTkjKUeDm
5LBESQpUzzUKNahZnpNDKKsgG5VgOuGfL/dziGm8i4XrQtUzJ3JGid7820iM9AuGkvL/NZuU3AX
PzJXYxtbH8wuuGG3hK7MM87VxUkN+302UxNcVKOABdg9Yk+QeqrgwZ6qJhECxn/coyFQZGBg98Q
EanYS94BLEoBk3jyiOTK4=
X-Gm-Gg: ASbGncvFpFsEFfHT/MpjJEJh06sYgzBR5wzTEAyjsb+sbgaD19hwtd+I5hjqWzu7Taz
pzbit76nI6dAQyjV2MK6S+XYzmB0Qo6s+jpjBXmym
X-Received: by 2002:a2e:b604:0:b0:302:251a:bd04 with SMTP id
38308e7fff4ca-30254438172mr31127461fa.9.1734343382879;
Mon, 16 Dec 2024 02:03:02 -0800 (PST)
X-Google-Smtp-Source: AGHT+IFZacWaM9PJDMFkq25haXSVhkUSFLUQWq+R+6B60FjeUvXLQnBizzlpJEmPNwiMVKc9EkaEPw6AJttNgUjrfNo=
X-Received: by 2002:a2e:b604:0:b0:302:251a:bd04 with SMTP id
38308e7fff4ca-30254438172mr31127391fa.9.1734343382471; Mon, 16 Dec 2024
02:03:02 -0800 (PST)
MIME-Version: 1.0
References: <20241205175754.1673888-1-david.marchand@redhat.com>
<20241205175754.1673888-3-david.marchand@redhat.com>
<16f6d334-d999-4fe7-b177-6daf881b8f1e@lysator.liu.se>
<CAJFAV8z2pCU+uk=Sv6Cpvt1_JBHjfV2zrNHWGCa5uEw1oohvjQ@mail.gmail.com>
<a76e3a84-57e6-42c9-97fe-bb1ebb02c210@lysator.liu.se>
In-Reply-To: <a76e3a84-57e6-42c9-97fe-bb1ebb02c210@lysator.liu.se>
From: David Marchand <david.marchand@redhat.com>
Date: Mon, 16 Dec 2024 11:02:51 +0100
Message-ID: <CAJFAV8zFTTQTV0ukAtPpkgZG2K0fqxTkbgKuQ4iY+kiwLKLzbw@mail.gmail.com>
Subject: Re: [PATCH 2/3] power: defer lcore variable allocation
To: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= <hofors@lysator.liu.se>
Cc: dev@dpdk.org, thomas@monjalon.net, frode.nordahl@canonical.com,
mattias.ronnblom@ericsson.com, stable@dpdk.org,
Anatoly Burakov <anatoly.burakov@intel.com>, David Hunt <david.hunt@intel.com>,
Sivaprasad Tummala <sivaprasad.tummala@amd.com>,
Stephen Hemminger <stephen@networkplumber.org>,
Chengwen Feng <fengchengwen@huawei.com>,
Konstantin Ananyev <konstantin.ananyev@huawei.com>,
=?UTF-8?Q?Morten_Br=C3=B8rup?= <mb@smartsharesystems.com>
X-Mimecast-Spam-Score: 0
X-Mimecast-MFC-PROC-ID: ADPv-Lv4mG_JBd52lqQ3G52gQ6N5vkJR69W2vfhOsCQ_1734343383
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
On Fri, Dec 13, 2024 at 7:58=E2=80=AFAM Mattias R=C3=B6nnblom <hofors@lysat=
or.liu.se> wrote:
> On 2024-12-12 08:57, David Marchand wrote:
> > clb_multiwait, clb_pause and clb_scale_freq callbacks can only be
> > reached after a successful call to
> > rte_power_ethdev_pmgmt_queue_enable.
> > Triggering an allocation in them means we are hiding a (internal)
> > programatic error as allocation and use of a lcore variable are
> > clearly separated atm.
> > If we keep the lcore var api as is, I would add an assert() (maybe
> > under a debug build option) in RTE_LCORE_VAR macros themselves, as
> > calling with a NULL handle means the initialisation path in some
> > code/RTE_LCORE_VAR API use was incorrect.
> >
>
> Sure, that would make sense. RTE_ASSERT(), that is. RTE_VERIFY() would
> be too expensive.
Yes, I'll send in next revision.
>
> >
> > Or because you propose to add the same type of helpers in both this
> > patch and the next, I am considering the other way: hide the
> > allocation in the RTE_LCORE_VAR* macros.
> > Checking for already allocated var in RTE_LCORE_VAR_ALLOC seems fine.
> > But the "fast path" RTE_LCORE_VAR would have an unwanted branch in most=
cases.
> >
>
> I would prefer to have the ALLOC() macro with semantics most people
> expect from a macro (or function) with that name, which is, I would
> argue, an unconditional allocation.
> It would make sense to have another macro, which performs an allocation
> only if the handle is NULL.
>
> RTE_LCORE_VAR_ASSURE_ALLOCATED(), or just RTE_LCORE_VAR_ASSURE()
> (although the latter sounds a little like an assertion, and not an
> allocation).
>
> RTE_LCORE_VAR_LAZY_ALLOC()
>
> I don't know. Something like that.
- In the power libary case, allocating the lcore variable is followed
by the initialisation of the lcore variable internals (per lcore
tailqs).
For this patch, I will rename the alloc_lcore_cfgs helper I had in v1 as:
static void
+init_lcore_cfgs(void)
+{
+ struct pmd_core_cfg *lcore_cfg;
+ unsigned int lcore_id;
+
+ if (lcore_cfgs !=3D NULL)
+ return;
+
+ RTE_LCORE_VAR_ALLOC(lcore_cfgs);
+
+ /* initialize all tailqs */
+ RTE_LCORE_VAR_FOREACH(lcore_id, lcore_cfg, lcore_cfgs)
+ TAILQ_INIT(&lcore_cfg->head);
+}
and only keep those checks in the public symbols.
- About more macros, I am wondering if this is needed in the end.
Adding assertions in the lcore var accessor should catch incorrect
initialisation path.
> >
> >> A somewhat unrelated question: why is pmd_core_cfg cache-line aligned?=
I
> >> don't think it should be.
> >
> > Before the conversion to per lcore variable, it was probably useful
> > (avoiding false sharing).
> > With the conversion, indeed, it looks like a waste of space.
> > It seems worth a separate fix.
> >
> >
>
> You will include it, or should I submit a separate patch?
I'll send it in next revision.
--=20
David Marchand