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 DBA49A050E; Fri, 15 Apr 2022 08:01:06 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 842C340140; Fri, 15 Apr 2022 08:01:06 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 504BA4003C for ; Fri, 15 Apr 2022 08:01:05 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 6054E20C353A; Thu, 14 Apr 2022 23:01:04 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 6054E20C353A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1650002464; bh=VZYGvLcG2PiUS1jv4fhJcrXGQfrqaY/H1y13dDoVwyI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Zggt3u3CjxQubJwCFV02tWMYNaz+AwsVegX+hw0N2U5h9Wk7yMO/nxTG1erxBYoJ4 xEzr4i+rAvoPhwMdJchtjgCI+OQVW7y7BbZt+MdnyVvQSTcBW4lvsMcATFedEpt5IT q4rMk6yMtJnZGKe2Gg0uTZpfQ18svWqCv4PjtSNs= Date: Thu, 14 Apr 2022 23:01:04 -0700 From: Tyler Retzlaff To: Dmitry Kozlyuk Cc: dev@dpdk.org, anatoly.burakov@intel.com Subject: Re: rte_memzone_reserve and invalid socket id Message-ID: <20220415060104.GA1823@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20220329060436.GA22196@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20220413075425.GA8292@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20220414220310.4eebc1dc@sovereign> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220414220310.4eebc1dc@sovereign> User-Agent: Mutt/1.5.21 (2010-09-15) 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 On Thu, Apr 14, 2022 at 10:03:10PM +0300, Dmitry Kozlyuk wrote: > 2022-04-13 00:54 (UTC-0700), Tyler Retzlaff: > > On Mon, Mar 28, 2022 at 11:04:36PM -0700, Tyler Retzlaff wrote: > [...] > > > memzone3 = rte_memzone_reserve(TEST_MEMZONE_NAME("testzone3"), 1000, > > > 1, 0); > > > ^ socket_id (to repeat just make it invalid) > > > > > > the parameter documentation provided for reference. > > > > > > * @param socket_id > > > * The socket identifier in the case of > > > * NUMA. The value can be SOCKET_ID_ANY if there is no NUMA > > > * constraint for the reserved zone. > > > > > > of interest is should rte_memzone_reserve fail when provided a > > > completely invalid socket_id? > > I think it should. > > > > > > > when running with --no-huge it does not because when --no-huge the > > > socket_id no matter the value is silently re-mapped to SOCKET_ID_ANY > > > though without --no-huge if a completely garbage socket_id were provided > > > it seems the allocation would fail. > > It's an implementation detail. > NUMA could be respected for --no-huge if there was a need. > > > > > > > so you get different behavior for an invalid socket_id depending on > > > --no-huge vs with. > > > > > > if (!rte_eal_has_hugepages() && socket_id < RTE_MAX_NUMA_NODES) > > > socket_id = SOCKET_ID_ANY; > > > > > > the test later fails at this check. where it compares the memzone3 > > > socket_id to what was used in the call to rte_memzone_reserve. > > > > > > if (memzone3 != NULL && memzone3->socket_id != 1) > > > return -1; ^ SOCKET_ID_ANY if --no-huge > > > > > > if the allocation had failed, the test would pass instead of failing at > > > this point. > > > > > > so what's wrong here? the test should be changed to expect different > > > behavior with --no-huge vs huge or should rte_memzone_reserve be > > > explicitly requiring SOCKET_ID_ANY instead of re-mapping invalid socket > > > id? > > memzone3->socket_id == SOCKET_ID_ANY should not be possible, > because it's a specific selected socket ID. > Rather, the check should be relaxed depending on rte_eal_has_hugepages(). okay, i think we are in agreement. my interpreted summary of how things should be are as follows. * rte_memzone_reserve() should fail if the provided socket_id is invalid for both huge and no-huge. * the test of memzone3 should be conditional on rte_eal_has_hugepages() instead of allocation succcess/failure of memzone3. note: if this is corrected it would have masked the bug/difference in behavior. * a separate test should exist that checks rte_memzone_reserve() correctly fails when given an invalid socket_id with both huge and no-huge. if the 3 points above were to be addressed the issue is it is a compatibility break. so while it should work a certain way the question for the community now what should we do? thanks