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 A14C9A00C4; Mon, 14 Nov 2022 23:13:57 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4A93A40DFD; Mon, 14 Nov 2022 23:13:57 +0100 (CET) Received: from mail-yb1-f171.google.com (mail-yb1-f171.google.com [209.85.219.171]) by mails.dpdk.org (Postfix) with ESMTP id 24E754014F for ; Mon, 14 Nov 2022 12:23:39 +0100 (CET) Received: by mail-yb1-f171.google.com with SMTP id o70so13029581yba.7 for ; Mon, 14 Nov 2022 03:23:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Sk9rFs0a3m2lEmU0T43NB5UoMSpO2dQBRj3FP2doIPc=; b=L+v3r7g7104pV02iRagj1teulikba4tfcixMSH2OZdQzA8BqXFAf/hwyn40KQaVQQR 64KbG0s7Tv0NWs39tkYD1PhP1v00Xi5K9qI3KVLbe7rbu+Y9WhVt4DM1nfZETfpDvcv5 snsMis70XbsmBMf4Y9dChQz+cdGH7aWXll4wE7gKzcoH7pDn/Nx89iku1+NzVkBi0dyv Kx2J6rJdSu6gThK4otOafNmn1yw97prWUjCwXvOhl80Pbj2mHI+a0R/XN0Jv/JL76YGD iCkn7v9uooMKI0RFsxi1k+bGDeXmAMWevEtngPIdH6m4aqOGmYSDlaXwlF3Loka3xOoV AR2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding: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=Sk9rFs0a3m2lEmU0T43NB5UoMSpO2dQBRj3FP2doIPc=; b=MMeVcTnIjruSzh3F0kY8ZT/8hcVJ1wVIRVoA2i4iWq/74NpAReFjYZqp+1ggm7TbGD bOEtN0oLhPI5LUZ82kioUm+47wM/dQ9OAcdU5fgMSALI04m4e5p08p30QGzmS5/wnUbd 7RL+sUO73DcZSHWwWJtZd/GiSkWFvW2StQXeMn5Wn6G0foj/8ad5ytvoJ9XSEvXf7CxZ bh8QT+NCX0mmnIRmveTLKKEGytZUp9JCvH4HxvCO5bhJywu4XrOORJlz1+uaAvK5+98S el+igRZNUSa55GFh5qtXlKWyqWacPeC60a7LcRekhEud+I61ogItVrCmXPRHdNz0sPL/ PDNg== X-Gm-Message-State: ANoB5pn158gyBZKO/3bSLA84/wwEaXwMFMGMEk/DiKAJb4kXatl2bNGa 4St7jNiau6k4CYWii/fH7zPZHytCPvBXsjNR0LiV+o2Y46vwag== X-Google-Smtp-Source: AA0mqf68b6vA/hb+WRriz/xxLca7ugJ+Ftzpvod4f3av2N/1K9ExIw89XbJBE5B38RLQC1UU76TMRwvdKnQykhmZs40= X-Received: by 2002:a25:bc91:0:b0:6cb:a739:bd25 with SMTP id e17-20020a25bc91000000b006cba739bd25mr12174784ybk.211.1668425019337; Mon, 14 Nov 2022 03:23:39 -0800 (PST) MIME-Version: 1.0 References: <20221114071439.38902-1-changfengnan@bytedance.com> In-Reply-To: <20221114071439.38902-1-changfengnan@bytedance.com> From: changfengnan Date: Mon, 14 Nov 2022 19:23:28 +0800 Message-ID: Subject: Re: [PATCH] mempool: fix rte_mempool_avail_count may segment fault when used in multiprocess To: olivier.matz@6wind.com, andrew.rybchenko@oktetlabs.ru, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Mon, 14 Nov 2022 23:13:56 +0100 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 It seems that this modification ignores some certain cases, for example cal= ling rte_mempool_create_empty directly, maybe add a new flag bit to indicate when to put tailq entry into rte_mempool_tailq list is a better way ? Looking forward to any suggestions. Thanks. Fengnan Chang. changfengnan =E4=BA=8E2022=E5=B9=B411=E6=9C=88= 14=E6=97=A5=E5=91=A8=E4=B8=80 15:14=E5=86=99=E9=81=93=EF=BC=9A > > rte_mempool_create put tailq entry into rte_mempool_tailq list before > populate, and pool_data set when populate. So in multi process, if > process A create mempool, and process B can get mempool through > rte_mempool_lookup before pool_data set, if B call rte_mempool_lookup, > it will cause segment fault. > Fix this by put tailq entry into rte_mempool_tailq after populate. > > Signed-off-by: changfengnan > --- > lib/mempool/rte_mempool.c | 40 +++++++++++++++++++-------------------- > 1 file changed, 19 insertions(+), 21 deletions(-) > > diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c > index 4c78071a34..b23d6138ff 100644 > --- a/lib/mempool/rte_mempool.c > +++ b/lib/mempool/rte_mempool.c > @@ -798,9 +798,7 @@ rte_mempool_create_empty(const char *name, unsigned n= , unsigned elt_size, > int socket_id, unsigned flags) > { > char mz_name[RTE_MEMZONE_NAMESIZE]; > - struct rte_mempool_list *mempool_list; > struct rte_mempool *mp =3D NULL; > - struct rte_tailq_entry *te =3D NULL; > const struct rte_memzone *mz =3D NULL; > size_t mempool_size; > unsigned int mz_flags =3D RTE_MEMZONE_1GB|RTE_MEMZONE_SIZE_HINT_O= NLY; > @@ -820,8 +818,6 @@ rte_mempool_create_empty(const char *name, unsigned n= , unsigned elt_size, > RTE_CACHE_LINE_MASK) !=3D 0); > #endif > > - mempool_list =3D RTE_TAILQ_CAST(rte_mempool_tailq.head, rte_mempo= ol_list); > - > /* asked for zero items */ > if (n =3D=3D 0) { > rte_errno =3D EINVAL; > @@ -866,14 +862,6 @@ rte_mempool_create_empty(const char *name, unsigned = n, unsigned elt_size, > private_data_size =3D (private_data_size + > RTE_MEMPOOL_ALIGN_MASK) & (~RTE_MEMPOOL_ALIG= N_MASK); > > - > - /* try to allocate tailq entry */ > - te =3D rte_zmalloc("MEMPOOL_TAILQ_ENTRY", sizeof(*te), 0); > - if (te =3D=3D NULL) { > - RTE_LOG(ERR, MEMPOOL, "Cannot allocate tailq entry!\n"); > - goto exit_unlock; > - } > - > mempool_size =3D RTE_MEMPOOL_HEADER_SIZE(mp, cache_size); > mempool_size +=3D private_data_size; > mempool_size =3D RTE_ALIGN_CEIL(mempool_size, RTE_MEMPOOL_ALIGN); > @@ -908,7 +896,6 @@ rte_mempool_create_empty(const char *name, unsigned n= , unsigned elt_size, > mp->private_data_size =3D private_data_size; > STAILQ_INIT(&mp->elt_list); > STAILQ_INIT(&mp->mem_list); > - > /* > * local_cache pointer is set even if cache_size is zero. > * The local_cache points to just past the elt_pa[] array. > @@ -922,12 +909,6 @@ rte_mempool_create_empty(const char *name, unsigned = n, unsigned elt_size, > mempool_cache_init(&mp->local_cache[lcore_id], > cache_size); > } > - > - te->data =3D mp; > - > - rte_mcfg_tailq_write_lock(); > - TAILQ_INSERT_TAIL(mempool_list, te, next); > - rte_mcfg_tailq_write_unlock(); > rte_mcfg_mempool_write_unlock(); > > rte_mempool_trace_create_empty(name, n, elt_size, cache_size, > @@ -936,7 +917,6 @@ rte_mempool_create_empty(const char *name, unsigned n= , unsigned elt_size, > > exit_unlock: > rte_mcfg_mempool_write_unlock(); > - rte_free(te); > rte_mempool_free(mp); > return NULL; > } > @@ -951,11 +931,22 @@ rte_mempool_create(const char *name, unsigned n, un= signed elt_size, > { > int ret; > struct rte_mempool *mp; > + struct rte_mempool_list *mempool_list; > + struct rte_tailq_entry *te =3D NULL; > + > + /* try to allocate tailq entry */ > + te =3D rte_zmalloc("MEMPOOL_TAILQ_ENTRY", sizeof(*te), 0); > + if (te =3D=3D NULL) { > + RTE_LOG(ERR, MEMPOOL, "Cannot allocate tailq entry!\n"); > + return NULL; > + } > > mp =3D rte_mempool_create_empty(name, n, elt_size, cache_size, > private_data_size, socket_id, flags); > - if (mp =3D=3D NULL) > + if (mp =3D=3D NULL) { > + rte_free(te); > return NULL; > + } > > /* > * Since we have 4 combinations of the SP/SC/MP/MC examine the fl= ags to > @@ -984,12 +975,19 @@ rte_mempool_create(const char *name, unsigned n, un= signed elt_size, > if (obj_init) > rte_mempool_obj_iter(mp, obj_init, obj_init_arg); > > + te->data =3D mp; > + mempool_list =3D RTE_TAILQ_CAST(rte_mempool_tailq.head, rte_mempo= ol_list); > + rte_mcfg_tailq_write_lock(); > + TAILQ_INSERT_TAIL(mempool_list, te, next); > + rte_mcfg_tailq_write_unlock(); > + > rte_mempool_trace_create(name, n, elt_size, cache_size, > private_data_size, mp_init, mp_init_arg, obj_init, > obj_init_arg, flags, mp); > return mp; > > fail: > + rte_free(te); > rte_mempool_free(mp); > return NULL; > } > -- > 2.37.0 (Apple Git-136) >