From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A2DFAA051C; Fri, 17 Jan 2020 21:27:59 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C6B2529D6; Fri, 17 Jan 2020 21:27:58 +0100 (CET) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id 31F9C1AFF for ; Fri, 17 Jan 2020 21:27:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579292876; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1hPJYHslQCBF/TsXhTE9hZpJTEDGMn1rNnBNlsU1fOg=; b=SPOKeIziP0FPNhl6Zi7EElAwijgZYxuoUkhDMuWcj6tNzCQCMFyxkIri3ExQ65FppkEAVt 8/XBBy/2yI/2UvoZZGXgfj3JiG2zbA/4IOL2QYuFWUjIsbMhlErXmSPkD5Cl2OWSLAiMjA b61CsLmUwGkSN0tKZClTpsiRtDm+Nuc= Received: from mail-vk1-f197.google.com (mail-vk1-f197.google.com [209.85.221.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-123-8OvwlAtYPK-PP0-dGq28Jw-1; Fri, 17 Jan 2020 15:27:54 -0500 Received: by mail-vk1-f197.google.com with SMTP id s205so10201372vka.17 for ; Fri, 17 Jan 2020 12:27:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=sI56tanK6B+uu2WT6s3njfPNVRycDpw8w2umg/bDWlQ=; b=MGj8PsLILA3s006oR19ldD7mxiWOuCGqILJ3b1vV7DJYvWP0qSlqmgZJLsM30tr93O 19S0w4bPP3GdZBkuS0BQTRVQ5aDZ2S30nY5G3QVNaXcXg1wSb+qfSWuPlg4QtklUShba uXtEOiTrQd7O/nv8+9UmMHjXwmQBD+8O04Q6bwTTiko9Nz+A05Ng7BSN3TbDCQ7vC0Vw lTJb2E3Da16XCORV/+eToLz5Hf8H16A7hEQQsZ6YvGao19qtQVegkWt7zm9SeeFTUXrW tuOA05SVW5oa8WcH5oQA1xFzL8U9lYIrjVQ48R5LgsGA6KpVd74NS7TMKH3/DVIl0Bo8 qTTg== X-Gm-Message-State: APjAAAX1/eq8oHm+vB03WePLrWYA7f0yGe9ndDHarFGDBjC4lQMBnL6c LnaLAzNXQxUB0UexILzJdYvNakTuuf+DAScfy6ybD4VALBGf5Ut1Fy2BFXDXu2qCVRPtf2Y2Ahc 4akATjj0mBvmYhWvswe8= X-Received: by 2002:a1f:4104:: with SMTP id o4mr151128vka.80.1579292874213; Fri, 17 Jan 2020 12:27:54 -0800 (PST) X-Google-Smtp-Source: APXvYqyZPjbsN5d+qR2WYpVoTjQwaBeUllrwmTP1fdR4zMpHfmEt9gmbZFNLl322flHneK0Feo5UtoVrAEDOcR6LGLA= X-Received: by 2002:a1f:4104:: with SMTP id o4mr151105vka.80.1579292873852; Fri, 17 Jan 2020 12:27:53 -0800 (PST) MIME-Version: 1.0 References: <20190906190510.11146-1-honnappa.nagarahalli@arm.com> <20200116052511.8557-1-honnappa.nagarahalli@arm.com> <20200116052511.8557-6-honnappa.nagarahalli@arm.com> In-Reply-To: <20200116052511.8557-6-honnappa.nagarahalli@arm.com> From: David Marchand Date: Fri, 17 Jan 2020 21:27:42 +0100 Message-ID: To: Honnappa Nagarahalli Cc: Olivier Matz , Stephen Hemminger , Jerin Jacob Kollanukkaran , Bruce Richardson , Pavan Nikhilesh , "Ananyev, Konstantin" , "Wang, Yipeng1" , dev , Dharmik Thakkar , "Ruifeng Wang (Arm Technology China)" , Gavin Hu , nd , Thomas Monjalon X-MC-Unique: 8OvwlAtYPK-PP0-dGq28Jw-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v9 5/6] lib/hash: use ring with 32b element size to save memory X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" On Thu, Jan 16, 2020 at 6:25 AM Honnappa Nagarahalli wrote: > > The freelist and external bucket indices are 32b. Using rings > that use 32b element sizes will save memory. > > Signed-off-by: Honnappa Nagarahalli > Reviewed-by: Gavin Hu > Reviewed-by: Ola Liljedahl > --- > lib/librte_hash/rte_cuckoo_hash.c | 94 ++++++++++++++++--------------- > lib/librte_hash/rte_cuckoo_hash.h | 2 +- > 2 files changed, 50 insertions(+), 46 deletions(-) > [snip] > diff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuck= oo_hash.h > index fb19bb27d..345de6bf9 100644 > --- a/lib/librte_hash/rte_cuckoo_hash.h > +++ b/lib/librte_hash/rte_cuckoo_hash.h > @@ -124,7 +124,7 @@ const rte_hash_cmp_eq_t cmp_jump_table[NUM_KEY_CMP_CA= SES] =3D { > > struct lcore_cache { > unsigned len; /**< Cache len */ > - void *objs[LCORE_CACHE_SIZE]; /**< Cache objects */ > + uint32_t objs[LCORE_CACHE_SIZE]; /**< Cache objects */ This triggers a warning in ABI checks: 1 function with some indirect sub-type change: [C]'function int32_t rte_hash_add_key(const rte_hash*, void*)' at rte_cuckoo_hash.c:1118:1 has some indirect sub-type changes: parameter 1 of type 'const rte_hash*' has sub-type changes: in pointed to type 'const rte_hash': in unqualified underlying type 'struct rte_hash' at rte_cuckoo_hash.h:160:1: type size hasn't changed 1 data member change: type of 'lcore_cache* rte_hash::local_free_slots' changed: in pointed to type 'struct lcore_cache' at rte_cuckoo_hash.h:1= 25:1: type size changed from 4608 to 2560 (in bits) 1 data member change: type of 'void* lcore_cache::objs[64]' changed: array element type 'void*' changed: entity changed from 'void*' to 'typedef uint32_t' at stdint-uintn.h:26:1 type size changed from 64 to 32 (in bits) type name changed from 'void*[64]' to 'uint32_t[64]' array type size changed from 4096 to 2048 and offset changed from 64 to 32 (in bits) (by -32 bits) As far as I can see, the local_free_slots field in rte_hash is supposed to be internal and should just be hidden from users. lib/librte_hash/rte_cuckoo_hash.c: h->local_free_slots =3D rte_zmalloc_socket(NULL, lib/librte_hash/rte_cuckoo_hash.c: rte_free(h->local_free_slot= s); lib/librte_hash/rte_cuckoo_hash.c: cached_cnt +=3D h->local_free_slots[i].len; lib/librte_hash/rte_cuckoo_hash.c: h->local_free_slots[i].len =3D 0; lib/librte_hash/rte_cuckoo_hash.c: cached_free_slots =3D &h->local_free_slots[lcore_id]; lib/librte_hash/rte_cuckoo_hash.c: cached_free_slots =3D &h->local_free_slots[lcore_id]; lib/librte_hash/rte_cuckoo_hash.c: cached_free_slots =3D &h->local_free_slots[lcore_id]; lib/librte_hash/rte_cuckoo_hash.h: struct lcore_cache *local_free_slot= s; Not sure how users could make use of this. But the abi check flags this as a breakage since this type was exported. I can see three options: - we stick to our "no abi breakage" policy, this change is postponed to the next ABI breakage, and at the same time, we hide this type and inspect the rest of the rte_hash API to avoid new issues in the future, - we duplicate structures and API by using function versioning to keep the exact rte_hash v20.0 ABI and a v20.0.1 ABI with the resized and cleaned structures, - we override the ABI freeze here by ruling that this was an internal structure that users should not access (ugh..) Seeing how this is an optimisation, my preference goes to the first option. --=20 David Marchand