From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-f193.google.com (mail-pf1-f193.google.com [209.85.210.193]) by dpdk.org (Postfix) with ESMTP id 737A31B56B for ; Fri, 5 Apr 2019 18:57:42 +0200 (CEST) Received: by mail-pf1-f193.google.com with SMTP id 10so3600182pfo.5 for ; Fri, 05 Apr 2019 09:57:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=O27mfYd8rsuduxVRC+rpmXS1f8sZvtYKOrZs71Ey8/g=; b=PViXlmKUKyfq6+toCYW0cCLxvLcwI0bHKupKAo0J4gVuzpLx4FvEGGS0B4pGtJaKEL pbQ3CUWQIzwpTo8VvzbMBMPePS0FsntkEExHg26uqnTFLS+usNMXfkRm/UQ1tHKj8qil etB9Tslop7cFIVArHcGtI+qTpNbBp7AGHNrYw3rq82bCwKkFv8+fYvz6sSENf2vTKn81 VMI5YJIR6Vb6VCxeuv4dxTagNV89ofeFWWoH8dc/K5fEyl+aGLno50CHkt6ZJz5nbgvE pm9Qdzn9rBVIc+4WG8etJMQjjwqGlqnb9hsKRRiPyJ0CDvV6Cwz4/w6U6SAPWmHw80By MWoQ== 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=O27mfYd8rsuduxVRC+rpmXS1f8sZvtYKOrZs71Ey8/g=; b=W26nQNMN8CO8wmNAPuKvDSQ7ZdBgHkDgSqoTlP7GOhGl+YKvEFvB5HMdAE48LcG9Yr 6C0y7WNtxMedHT7sGteYjZ/iuVqJixr9n50mIW+QuiLsWFO8/PpsNH7s81MachOR28Wv ASB4xRJJ9b0SHWi0ND2l2kHsvLPtIXWfOlL5jVQsNNWqoSBaqM4Lk0ddinjMU+zDJn9Z v9TYOUoBOxmSjaG3VoKhfI0av7VveahPIJcACs1e3DiXIMMeYjrDEz71jtG9+3W5qbDd XJWLtXhK4n5R3pc2NNrj6gCVyaGum1vGa52xgjt+9dvvuw2jnm7ym+Vg4E7SjYyIrcQ3 /b/g== X-Gm-Message-State: APjAAAWUJvWgcj0S165+uTZ//gTuGhM8njWfTWNhVBJ7iOVvSNV46cPr xHamWIUaG1icLbagHoiXgc/SSg== X-Google-Smtp-Source: APXvYqxGN45yn7iMLjXCuHRerPPxPcbWVmtVWrboOmAp613Q0pqtI7Y5fYt0zkZh8tUSJWWacBgneQ== X-Received: by 2002:a63:5b0a:: with SMTP id p10mr13284741pgb.282.1554483461340; Fri, 05 Apr 2019 09:57:41 -0700 (PDT) Received: from shemminger-XPS-13-9360 ([167.220.56.255]) by smtp.gmail.com with ESMTPSA id e123sm35587869pgc.14.2019.04.05.09.57.41 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 05 Apr 2019 09:57:41 -0700 (PDT) Date: Fri, 5 Apr 2019 09:57:38 -0700 From: Stephen Hemminger To: Mattias =?UTF-8?B?UsO2bm5ibG9t?= Cc: Message-ID: <20190405095738.145a622d@shemminger-XPS-13-9360> In-Reply-To: <20190405134542.28618-1-mattias.ronnblom@ericsson.com> References: <20190405134542.28618-1-mattias.ronnblom@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [RFC] eal: make rte_rand() MT safe 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: , X-List-Received-Date: Fri, 05 Apr 2019 16:57:42 -0000 On Fri, 5 Apr 2019 15:45:42 +0200 Mattias R=C3=B6nnblom wrote: > The rte_rand() documentation left it unspecified if the rte_rand() was > multi-thread safe or not, and the implementation (based on lrand48()) > was not. >=20 > This commit makes rte_rand() safe to use from any lcore thread by > using lrand48_r() and per-lcore random state structs. Besides the > obvious improvement in terms of correctness (for concurrent users), > this also much improves rte_rand() performance, since the threads no > longer shares state. For the single-threaded case, this patch causes > ~10% rte_rand() performance degradation. >=20 > rte_srand() is left multi-thread unsafe, and external synchronization > is required to serialize rte_sand() calls from different lcore > threads, and a rte_srand() call with rte_rand() calls made by other > lcore threads. >=20 > The assumption is that the random number generators will be seeded > once, during startup. >=20 > Bugzilla ID: 114 >=20 > Signed-off-by: Mattias R=C3=B6nnblom > --- > lib/librte_eal/common/include/rte_random.h | 25 ++++----- > lib/librte_eal/common/meson.build | 1 + > lib/librte_eal/common/rte_random.c | 65 ++++++++++++++++++++++ > lib/librte_eal/freebsd/eal/Makefile | 1 + > lib/librte_eal/freebsd/eal/eal.c | 2 - > lib/librte_eal/linux/eal/Makefile | 1 + > lib/librte_eal/linux/eal/eal.c | 2 - > lib/librte_eal/rte_eal_version.map | 2 + > 8 files changed, 80 insertions(+), 19 deletions(-) > create mode 100644 lib/librte_eal/common/rte_random.c >=20 > diff --git a/lib/librte_eal/common/include/rte_random.h b/lib/librte_eal/= common/include/rte_random.h > index b2ca1c209..bca85a672 100644 > --- a/lib/librte_eal/common/include/rte_random.h > +++ b/lib/librte_eal/common/include/rte_random.h > @@ -16,7 +16,6 @@ extern "C" { > #endif > =20 > #include > -#include > =20 > /** > * Seed the pseudo-random generator. > @@ -25,14 +24,15 @@ extern "C" { > * value. It may need to be re-seeded by the user with a real random > * value. > * > + * This function is not multi-thread safe in regards to other > + * rte_srand() calls, nor is it in relation to concurrent rte_rand() > + * calls. > + * > * @param seedval > * The value of the seed. > */ > -static inline void > -rte_srand(uint64_t seedval) > -{ > - srand48((long)seedval); > -} > +void > +rte_srand(uint64_t seedval); > =20 > /** > * Get a pseudo-random value. > @@ -41,18 +41,13 @@ rte_srand(uint64_t seedval) > * congruential algorithm and 48-bit integer arithmetic, called twice > * to generate a 64-bit value. > * > + * If called from lcore threads, this function is thread-safe. > + * > * @return > * A pseudo-random value between 0 and (1<<64)-1. > */ > -static inline uint64_t > -rte_rand(void) > -{ > - uint64_t val; > - val =3D (uint64_t)lrand48(); > - val <<=3D 32; > - val +=3D (uint64_t)lrand48(); > - return val; > -} > +uint64_t > +rte_rand(void); > =20 > #ifdef __cplusplus > } > diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/me= son.build > index 0670e4102..bafd23207 100644 > --- a/lib/librte_eal/common/meson.build > +++ b/lib/librte_eal/common/meson.build > @@ -35,6 +35,7 @@ common_sources =3D files( > 'rte_keepalive.c', > 'rte_malloc.c', > 'rte_option.c', > + 'rte_random.c', > 'rte_reciprocal.c', > 'rte_service.c' > ) > diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/r= te_random.c > new file mode 100644 > index 000000000..9d519d03b > --- /dev/null > +++ b/lib/librte_eal/common/rte_random.c > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2010-2014 Intel Corporation > + * Copyright(c) 2019 Ericsson AB > + */ > + > +#include > + > +#include > +#include > +#include > +#include > + > +struct rte_rand_data > +{ > + struct drand48_data data; > +} __rte_cache_aligned; > + > +static struct rte_rand_data rand_data[RTE_MAX_LCORE]; > + > +void > +rte_srand(uint64_t seedval) > +{ > + unsigned i; > + > + /* give the different lcores a different seed, to avoid a > + situation where they generate the same sequence */ > + for (i =3D 0; i < RTE_MAX_LCORE; i++) > + srand48_r((long)seedval + i, &rand_data[i].data); > +} > + > +static inline uint32_t > +__rte_rand48(struct drand48_data *data) > +{ > + long res; > + > + lrand48_r(data, &res); > + > + return (uint32_t)res; > +} > + > +uint64_t > +rte_rand(void) > +{ > + unsigned lcore_id; > + struct drand48_data *data; > + uint64_t val; > + > + lcore_id =3D rte_lcore_id(); > + > + if (unlikely(lcore_id =3D=3D LCORE_ID_ANY)) > + lcore_id =3D rte_get_master_lcore(); > + > + data =3D &rand_data[lcore_id].data; > + > + val =3D __rte_rand48(data); > + val <<=3D 32; > + val +=3D __rte_rand48(data); > + > + return val; > +} > + > +RTE_INIT(rte_rand_init) > +{ > + rte_srand(rte_get_timer_cycles()); > +} rand48 is a terrible PRNG, why not use something better? Similar discussion in Linux kernel pointed at: http://www.pcg-random.org/posts/some-prng-implementations.html Mail thread here: https://www.spinics.net/lists/netdev/msg560231.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 19DE8A0679 for ; Fri, 5 Apr 2019 18:57:47 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 26F791B572; Fri, 5 Apr 2019 18:57:44 +0200 (CEST) Received: from mail-pf1-f193.google.com (mail-pf1-f193.google.com [209.85.210.193]) by dpdk.org (Postfix) with ESMTP id 737A31B56B for ; Fri, 5 Apr 2019 18:57:42 +0200 (CEST) Received: by mail-pf1-f193.google.com with SMTP id 10so3600182pfo.5 for ; Fri, 05 Apr 2019 09:57:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=O27mfYd8rsuduxVRC+rpmXS1f8sZvtYKOrZs71Ey8/g=; b=PViXlmKUKyfq6+toCYW0cCLxvLcwI0bHKupKAo0J4gVuzpLx4FvEGGS0B4pGtJaKEL pbQ3CUWQIzwpTo8VvzbMBMPePS0FsntkEExHg26uqnTFLS+usNMXfkRm/UQ1tHKj8qil etB9Tslop7cFIVArHcGtI+qTpNbBp7AGHNrYw3rq82bCwKkFv8+fYvz6sSENf2vTKn81 VMI5YJIR6Vb6VCxeuv4dxTagNV89ofeFWWoH8dc/K5fEyl+aGLno50CHkt6ZJz5nbgvE pm9Qdzn9rBVIc+4WG8etJMQjjwqGlqnb9hsKRRiPyJ0CDvV6Cwz4/w6U6SAPWmHw80By MWoQ== 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=O27mfYd8rsuduxVRC+rpmXS1f8sZvtYKOrZs71Ey8/g=; b=W26nQNMN8CO8wmNAPuKvDSQ7ZdBgHkDgSqoTlP7GOhGl+YKvEFvB5HMdAE48LcG9Yr 6C0y7WNtxMedHT7sGteYjZ/iuVqJixr9n50mIW+QuiLsWFO8/PpsNH7s81MachOR28Wv ASB4xRJJ9b0SHWi0ND2l2kHsvLPtIXWfOlL5jVQsNNWqoSBaqM4Lk0ddinjMU+zDJn9Z v9TYOUoBOxmSjaG3VoKhfI0av7VveahPIJcACs1e3DiXIMMeYjrDEz71jtG9+3W5qbDd XJWLtXhK4n5R3pc2NNrj6gCVyaGum1vGa52xgjt+9dvvuw2jnm7ym+Vg4E7SjYyIrcQ3 /b/g== X-Gm-Message-State: APjAAAWUJvWgcj0S165+uTZ//gTuGhM8njWfTWNhVBJ7iOVvSNV46cPr xHamWIUaG1icLbagHoiXgc/SSg== X-Google-Smtp-Source: APXvYqxGN45yn7iMLjXCuHRerPPxPcbWVmtVWrboOmAp613Q0pqtI7Y5fYt0zkZh8tUSJWWacBgneQ== X-Received: by 2002:a63:5b0a:: with SMTP id p10mr13284741pgb.282.1554483461340; Fri, 05 Apr 2019 09:57:41 -0700 (PDT) Received: from shemminger-XPS-13-9360 ([167.220.56.255]) by smtp.gmail.com with ESMTPSA id e123sm35587869pgc.14.2019.04.05.09.57.41 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 05 Apr 2019 09:57:41 -0700 (PDT) Date: Fri, 5 Apr 2019 09:57:38 -0700 From: Stephen Hemminger To: Mattias =?UTF-8?B?UsO2bm5ibG9t?= Cc: Message-ID: <20190405095738.145a622d@shemminger-XPS-13-9360> In-Reply-To: <20190405134542.28618-1-mattias.ronnblom@ericsson.com> References: <20190405134542.28618-1-mattias.ronnblom@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [RFC] eal: make rte_rand() MT safe 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" Message-ID: <20190405165738.Jd-oBpE0QIJO_PEK7P2067yLWLX6Mzs8-bfnCE-TQQo@z> On Fri, 5 Apr 2019 15:45:42 +0200 Mattias R=C3=B6nnblom wrote: > The rte_rand() documentation left it unspecified if the rte_rand() was > multi-thread safe or not, and the implementation (based on lrand48()) > was not. >=20 > This commit makes rte_rand() safe to use from any lcore thread by > using lrand48_r() and per-lcore random state structs. Besides the > obvious improvement in terms of correctness (for concurrent users), > this also much improves rte_rand() performance, since the threads no > longer shares state. For the single-threaded case, this patch causes > ~10% rte_rand() performance degradation. >=20 > rte_srand() is left multi-thread unsafe, and external synchronization > is required to serialize rte_sand() calls from different lcore > threads, and a rte_srand() call with rte_rand() calls made by other > lcore threads. >=20 > The assumption is that the random number generators will be seeded > once, during startup. >=20 > Bugzilla ID: 114 >=20 > Signed-off-by: Mattias R=C3=B6nnblom > --- > lib/librte_eal/common/include/rte_random.h | 25 ++++----- > lib/librte_eal/common/meson.build | 1 + > lib/librte_eal/common/rte_random.c | 65 ++++++++++++++++++++++ > lib/librte_eal/freebsd/eal/Makefile | 1 + > lib/librte_eal/freebsd/eal/eal.c | 2 - > lib/librte_eal/linux/eal/Makefile | 1 + > lib/librte_eal/linux/eal/eal.c | 2 - > lib/librte_eal/rte_eal_version.map | 2 + > 8 files changed, 80 insertions(+), 19 deletions(-) > create mode 100644 lib/librte_eal/common/rte_random.c >=20 > diff --git a/lib/librte_eal/common/include/rte_random.h b/lib/librte_eal/= common/include/rte_random.h > index b2ca1c209..bca85a672 100644 > --- a/lib/librte_eal/common/include/rte_random.h > +++ b/lib/librte_eal/common/include/rte_random.h > @@ -16,7 +16,6 @@ extern "C" { > #endif > =20 > #include > -#include > =20 > /** > * Seed the pseudo-random generator. > @@ -25,14 +24,15 @@ extern "C" { > * value. It may need to be re-seeded by the user with a real random > * value. > * > + * This function is not multi-thread safe in regards to other > + * rte_srand() calls, nor is it in relation to concurrent rte_rand() > + * calls. > + * > * @param seedval > * The value of the seed. > */ > -static inline void > -rte_srand(uint64_t seedval) > -{ > - srand48((long)seedval); > -} > +void > +rte_srand(uint64_t seedval); > =20 > /** > * Get a pseudo-random value. > @@ -41,18 +41,13 @@ rte_srand(uint64_t seedval) > * congruential algorithm and 48-bit integer arithmetic, called twice > * to generate a 64-bit value. > * > + * If called from lcore threads, this function is thread-safe. > + * > * @return > * A pseudo-random value between 0 and (1<<64)-1. > */ > -static inline uint64_t > -rte_rand(void) > -{ > - uint64_t val; > - val =3D (uint64_t)lrand48(); > - val <<=3D 32; > - val +=3D (uint64_t)lrand48(); > - return val; > -} > +uint64_t > +rte_rand(void); > =20 > #ifdef __cplusplus > } > diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/me= son.build > index 0670e4102..bafd23207 100644 > --- a/lib/librte_eal/common/meson.build > +++ b/lib/librte_eal/common/meson.build > @@ -35,6 +35,7 @@ common_sources =3D files( > 'rte_keepalive.c', > 'rte_malloc.c', > 'rte_option.c', > + 'rte_random.c', > 'rte_reciprocal.c', > 'rte_service.c' > ) > diff --git a/lib/librte_eal/common/rte_random.c b/lib/librte_eal/common/r= te_random.c > new file mode 100644 > index 000000000..9d519d03b > --- /dev/null > +++ b/lib/librte_eal/common/rte_random.c > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2010-2014 Intel Corporation > + * Copyright(c) 2019 Ericsson AB > + */ > + > +#include > + > +#include > +#include > +#include > +#include > + > +struct rte_rand_data > +{ > + struct drand48_data data; > +} __rte_cache_aligned; > + > +static struct rte_rand_data rand_data[RTE_MAX_LCORE]; > + > +void > +rte_srand(uint64_t seedval) > +{ > + unsigned i; > + > + /* give the different lcores a different seed, to avoid a > + situation where they generate the same sequence */ > + for (i =3D 0; i < RTE_MAX_LCORE; i++) > + srand48_r((long)seedval + i, &rand_data[i].data); > +} > + > +static inline uint32_t > +__rte_rand48(struct drand48_data *data) > +{ > + long res; > + > + lrand48_r(data, &res); > + > + return (uint32_t)res; > +} > + > +uint64_t > +rte_rand(void) > +{ > + unsigned lcore_id; > + struct drand48_data *data; > + uint64_t val; > + > + lcore_id =3D rte_lcore_id(); > + > + if (unlikely(lcore_id =3D=3D LCORE_ID_ANY)) > + lcore_id =3D rte_get_master_lcore(); > + > + data =3D &rand_data[lcore_id].data; > + > + val =3D __rte_rand48(data); > + val <<=3D 32; > + val +=3D __rte_rand48(data); > + > + return val; > +} > + > +RTE_INIT(rte_rand_init) > +{ > + rte_srand(rte_get_timer_cycles()); > +} rand48 is a terrible PRNG, why not use something better? Similar discussion in Linux kernel pointed at: http://www.pcg-random.org/posts/some-prng-implementations.html Mail thread here: https://www.spinics.net/lists/netdev/msg560231.html