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 6DB44A0C43; Wed, 20 Oct 2021 17:30:54 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 36E9C411BB; Wed, 20 Oct 2021 17:30:54 +0200 (CEST) Received: from mail-lj1-f176.google.com (mail-lj1-f176.google.com [209.85.208.176]) by mails.dpdk.org (Postfix) with ESMTP id 6F63741196 for ; Wed, 20 Oct 2021 17:30:53 +0200 (CEST) Received: by mail-lj1-f176.google.com with SMTP id o26so13312341ljj.2 for ; Wed, 20 Oct 2021 08:30:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=GakdGBB+Swjoz+48GiyaU/6JvbUXz2rM+2EG1F8rD3E=; b=LIO4ifvLMIMZy+hoVt2VBEb6CMArfuF6HZ1aQ15vErZHycW6v46uZfdZlv5mWcqRkM lrOlx/wp+XVOjnZhf1rdBCg5GklHmAWND4IusHh18guhy1WU6pFyfY30nRBR/MwkegD0 ZNVeQZe51WPAGcbqTyxqqS3Lwhu+3q6UgQ3UwxV/4x64v9yAwgyrHN90d0OUhijS1PMe f40rzWzm6+G7TGIJghSrBKNastBwqS23Qth/I/xNHK0xPufrrtvndLeryiemWGFil6fU cs6ejn9b4xPm5HYwiDeAq8h2r3Z1L02aTO8LNkoECGLyuxdYA0FGPBIlAOR5NVMwNkAV As1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=GakdGBB+Swjoz+48GiyaU/6JvbUXz2rM+2EG1F8rD3E=; b=MrDnIMTs/e6zDWBjX2ONAz3T/lHy8Zz41L0hdYkyNWnzLoE4Zln3QfN8czDOXtWU78 OfXno/UNFs/6LinFs49ibM+5F7YxELRVu/DhyjZZUTJumyfoozHAZIHhQahh1OB7WwOY Xwdiqk4XhSv2ugIoO9kNv4yuf6ffUA73ACdMXE/4jJWQMy0aiqVPKITcNpsTYbZtri3T ubWDLGsWUE1zfetKyDI71ZU8ozG1JJt6RDdwuq/jKSfcZqgGecXEQItnI3mhTikICOQr DZ1DO0NaObgeIpYffebIqr1spFw4CP8aoszYZrdOKRzQMVRjW+LRbw39gpWlLpC04vGE BHEA== X-Gm-Message-State: AOAM531DKPeo3VqrWH5i8bzFV7ne3xN+8P6ThH2Novgo8wKD0GLbxoZ+ Y4vbyPMi0Rq04m+oTOGYosc= X-Google-Smtp-Source: ABdhPJxTghplSamAennwl4QudoRq2TYzPHE8Qi9mHVtN63UKCZanRa/oSBeogeS54VpJMAPvmEH8KQ== X-Received: by 2002:a2e:9c49:: with SMTP id t9mr139440ljj.86.1634743852932; Wed, 20 Oct 2021 08:30:52 -0700 (PDT) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id s4sm213856lfs.235.2021.10.20.08.30.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Oct 2021 08:30:52 -0700 (PDT) Date: Wed, 20 Oct 2021 18:30:51 +0300 From: Dmitry Kozlyuk To: Harman Kalra Cc: Stephen Hemminger , Thomas Monjalon , "david.marchand@redhat.com" , "dev@dpdk.org" , Ray Kinsella Message-ID: <20211020183051.657b05c1@sovereign> In-Reply-To: References: <20210826145726.102081-1-hkalra@marvell.com> <20211018193707.123559-1-hkalra@marvell.com> <20211018193707.123559-3-hkalra@marvell.com> <20211018155654.0d3ffbed@hermes.local> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v3 2/7] eal/interrupts: implement get set APIs 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 Sender: "dev" 2021-10-19 08:32 (UTC+0000), Harman Kalra: > > -----Original Message----- > > From: Stephen Hemminger > > Sent: Tuesday, October 19, 2021 4:27 AM > > To: Harman Kalra > > Cc: dev@dpdk.org; Thomas Monjalon ; Ray Kinsella > > ; david.marchand@redhat.com; > > dmitry.kozliuk@gmail.com > > Subject: [EXT] Re: [dpdk-dev] [PATCH v3 2/7] eal/interrupts: implement get > > set APIs > > > > External Email > > > > ---------------------------------------------------------------------- > > On Tue, 19 Oct 2021 01:07:02 +0530 > > Harman Kalra wrote: > > > > > + /* Detect if DPDK malloc APIs are ready to be used. */ > > > + mem_allocator = rte_malloc_is_ready(); > > > + if (mem_allocator) > > > + intr_handle = rte_zmalloc(NULL, sizeof(struct > > rte_intr_handle), > > > + 0); > > > + else > > > + intr_handle = calloc(1, sizeof(struct rte_intr_handle)); > > > > This is problematic way to do this. > > The reason to use rte_malloc vs malloc should be determined by usage. > > > > If the pointer will be shared between primary/secondary process then it has > > to be in hugepages (ie rte_malloc). If it is not shared then then use regular > > malloc. > > > > But what you have done is created a method which will be a latent bug for > > anyone using primary/secondary process. > > > > Either: > > intr_handle is not allowed to be used in secondary. > > Then always use malloc(). > > Or. > > intr_handle can be used by both primary and secondary. > > Then always use rte_malloc(). > > Any code path that allocates intr_handle before pool is > > ready is broken. > > Hi Stephan, > > Till V2, I implemented this API in a way where user of the API can choose > If he wants intr handle to be allocated using malloc or rte_malloc by passing > a flag arg to the rte_intr_instanc_alloc API. User of the API will best know if > the intr handle is to be shared with secondary or not. > > But after some discussions and suggestions from the community we decided > to drop that flag argument and auto detect on whether rte_malloc APIs are > ready to be used and thereafter make all further allocations via rte_malloc. > Currently alarm subsystem (or any driver doing allocation in constructor) gets > interrupt instance allocated using glibc malloc that too because rte_malloc* > is not ready by rte_eal_alarm_init(), while all further consumers gets instance > allocated via rte_malloc. Just as a comment, bus scanning is the real issue, not the alarms. Alarms could be initialized after the memory management (but it's irrelevant because their handle is not accessed from the outside). However, MM needs to know bus IOVA requirements to initialize, which is usually determined by at least bus device requirements. > I think this should not cause any issue in primary/secondary model as all interrupt > instance pointer will be shared. What do you mean? Aren't we discussing the issue that those allocated early are not shared? > Infact to avoid any surprises of primary/secondary > not working we thought of making all allocations via rte_malloc. I don't see why anyone would not make them shared. In order to only use rte_malloc(), we need: 1. In bus drivers, move handle allocation from scan to probe stage. 2. In EAL, move alarm initialization to after the MM. It all can be done later with v3 design---but there are out-of-tree drivers. We need to force them to make step 1 at some point. I see two options: a) Right now have an external API that only works with rte_malloc() and internal API with autodetection. Fix DPDK and drop internal API. b) Have external API with autodetection. Fix DPDK. At the next ABI breakage drop autodetection and libc-malloc. > David, Thomas, Dmitry, please add if I missed anything. > > Can we please conclude on this series APIs as API freeze deadline (rc1) is very near. I support v3 design with no options and autodetection, because that's the interface we want in the end. Implementation can be improved later.