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 4CB35A0547; Tue, 15 Nov 2022 12:33:31 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D5E5442D13; Tue, 15 Nov 2022 12:33:20 +0100 (CET) Received: from mail-yb1-f174.google.com (mail-yb1-f174.google.com [209.85.219.174]) by mails.dpdk.org (Postfix) with ESMTP id A461A40DFD for ; Tue, 15 Nov 2022 12:30:17 +0100 (CET) Received: by mail-yb1-f174.google.com with SMTP id z192so16884421yba.0 for ; Tue, 15 Nov 2022 03:30:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=J08iagek9vjPPXB6xWzz9Wh1y649CIawivtc56Nt2Gw=; b=W+3yacVAr1jKIUFBtnBWiS+Uy9vnlAnMAAJt0iw3A6y4JmfhIWhZPqPh91hluc1iIU HiaG84KV4Njg4u68za8vQAEC/VwJ1/7SMRJDs7mrquSO85xwuxpJPc8N8OvZlXowjfwA 2oYnXtzvC/5pUNmi2+5IWTEwJj5YVUafJ3KD7oSvhidJpyTgVSwfuLQFXsfy//pp8WqE Novy9H+LPqevOzLFoStXfOwA0u6TlG2q0kRjihWzu0cvvocY2KqN7zIzgDbY8oSZhTtO N0V6CkgWTBak8YNLyHU/SAPNWmvpJrtIuDYF+ONZ3jWRMZc9INjEla+Tc7HK507G4em+ CoVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=J08iagek9vjPPXB6xWzz9Wh1y649CIawivtc56Nt2Gw=; b=r4t2JlExELJiMTeUs4h/dZlKOgtf5xqI/was1qhLJ/M0sLLuVI6VLIyWYQfBVD7aJq yRYyqL4V0kWGs2F3XoI7IIWPcP2FxNiK13C5Lf+H9SxiKvACK0TBjpSnzYzt6Qbgyms6 1ImLXOmHW8WiaApoNlXlp73JEIY8JiyTs1Z9v5Q9q0P1Ac4irLOp9KxwEVy25RzDX0G6 gdIqCQB4ymRvhSAfzQE730D02nX7VB4uPJuQIG3OKuZHjmYmgw3KDvfau1JiaUDJ45MB FhalrQYPE7YqVI4Z4JRpUYYnVAFwBoDf516jqcwV/389n7m5MsQR0VoBwsoi8qgYcqRh APSw== X-Gm-Message-State: ANoB5plMUsXehWBW+AU9x/0cD/8HENmXIWgeR/RWlYNlHVOLyXEEwWRw AAO4+zI9JmHo46emopXIGCt6XFwkdAOk22VFwc4ywEBDhqMeeEeZ X-Google-Smtp-Source: AA0mqf66ucflRfy7QbgOkbsNsuAPzG3GGxETrFTWCxW2UQUqqhnkUiKObTtvwOe18WISFv+J+1sVh09+ix5mSyHxig0= X-Received: by 2002:a25:bc91:0:b0:6cb:a739:bd25 with SMTP id e17-20020a25bc91000000b006cba739bd25mr16757772ybk.211.1668511816745; Tue, 15 Nov 2022 03:30:16 -0800 (PST) MIME-Version: 1.0 References: <20221114071439.38902-1-changfengnan@bytedance.com> In-Reply-To: From: Fengnan Chang Date: Tue, 15 Nov 2022 19:30:05 +0800 Message-ID: Subject: Re: [External] Re: [PATCH] mempool: fix rte_mempool_avail_count may segment fault when used in multiprocess To: Olivier Matz Cc: David Marchand , andrew.rybchenko@oktetlabs.ru, dev@dpdk.org, =?UTF-8?Q?Morten_Br=C3=B8rup?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Tue, 15 Nov 2022 12:33:18 +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 Olivier Matz =E4=BA=8E2022=E5=B9=B411=E6=9C=8815= =E6=97=A5=E5=91=A8=E4=BA=8C 16:29=E5=86=99=E9=81=93=EF=BC=9A > > Hi, > > On Tue, Nov 15, 2022 at 08:47:15AM +0100, David Marchand wrote: > > On Tue, Nov 15, 2022 at 2:51 AM Fengnan Chang > > wrote: > > > > > > David Marchand =E4=BA=8E2022=E5=B9=B411= =E6=9C=8815=E6=97=A5=E5=91=A8=E4=BA=8C 04:44=E5=86=99=E9=81=93=EF=BC=9A > > > > > > > > On Mon, Nov 14, 2022 at 9:13 AM changfengnan wrote: > > > > > > > > > > rte_mempool_create put tailq entry into rte_mempool_tailq list be= fore > > > > > populate, and pool_data set when populate. So in multi process, i= f > > > > > process A create mempool, and process B can get mempool through > > > > > rte_mempool_lookup before pool_data set, if B call rte_mempool_lo= okup, > > > > > it will cause segment fault. > > > > > > > > I fail to see how pool_data impacts rte_mempool_lookup. > > > > Something is fishy about this commitlog. > > > > > > oh, it's my fault about this commit. correct: if B can get mempool th= rough > > > rte_mempool_lookup before pool_data set, and call rte_mempool_avail_c= ount, > > > it will cause segment fault. > > > > Ok, now it makes more sense :-). > > > > > > > > > > > > > > > > > > Fix this by put tailq entry into rte_mempool_tailq after populate= . > > > > > > > > Moving tailq manipulation to rte_mempool_create only, is probably i= ncorrect. > > > > An application is allowed to call rte_mempool_create_empty() and > > > > rte_mempool_populate(). > > > > > > > > I did not look in depth, but It is likely the reason why testpmd (a= s > > > > run with devtools/test-null.sh) won't pass anymore. > > > > The CI reported this issue in various envs. > > > > > > > > We can't take this patch. > > > > > > Yeah, this version makes CI fail. > > > I didn't notice rte_mempool_create_empty will called directly before,= maybe > > > add a new flag bit to indicate when to put tailq entry into rte_mempo= ol_tailq > > > list is a better way. If no better idea, I'll send a new version. > > > > I don't think we need an other flag. > > Can we "publish" the mempool at the mempool_ops_alloc_once stage? > > The mempool_ops_alloc_once() seems it is the proper place, yes. > > Alternatively, I suppose this issue can be fixed in the secondary > application: I found this problem in spdk, if this is fixed in the secondary app, it needs modify spdk too. The following options work, but in spdk, I prefer to publish the mempool a= t mempool_ops_alloc_once, I'll send a new version like this. > > - it can wait that the flag RTE_MEMPOOL_F_POOL_CREATED is present before > using the mempool. > > - or it can wait the RTE_MEMPOOL_EVENT_READY > > - or it can wait that the whole initialization of the primary > application is finished by another mean (a sort of lock). I don't know > the exact use case, but to me, it looks sane to do that, it would > protect from other similar issues. > > > Olivier > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: changfengnan > > > > > > > > Please use your real name. > > > > > > It's my real name. > > > > Sorry, I meant your full name, like Fengnan Chang > > > > > > -- > > David Marchand > >