Olivier Matz <olivier.m...@6wind.com> 于2022年11月15日周二 16:29写道： > > 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 > > <changfeng...@bytedance.com> wrote: > > > > > > David Marchand <david.march...@redhat.com> 于2022年11月15日周二 04:44写道： > > > > > > > > On Mon, Nov 14, 2022 at 9:13 AM changfengnan > > > > <changfeng...@bytedance.com> wrote: > > > > > > > > > > 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. > > > > > > > > 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 through > > > rte_mempool_lookup before pool_data set, and call rte_mempool_avail_count, > > > 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 > > > > incorrect. > > > > 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 (as > > > > 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_mempool_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 at 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 <changfeng...@bytedance.com> > > > > > > > > Please use your real name. > > > > > > It's my real name. > > > > Sorry, I meant your full name, like Fengnan Chang > > <changfeng...@bytedance.com> > > > > > > -- > > David Marchand > >
