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 9CAA3A059F; Fri, 10 Apr 2020 12:34:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0DCF21C2AF; Fri, 10 Apr 2020 12:34:17 +0200 (CEST) Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2052.outbound.protection.outlook.com [40.107.21.52]) by dpdk.org (Postfix) with ESMTP id 05E371C2A4 for ; Fri, 10 Apr 2020 12:34:15 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dNi7WKaWoLLFFdNPAjiuWueFtbvgoK6EbDtw5rXtup0j/4Ti2u2tFRJbg9ebPpbvsXkOUN6d5eysY4HQR+UbqkI0yYzBz8VSfYTH2bdVRpHEk76BpeGTZZWYDCJGslv72wM9/ENt15fYpFNjgET1mIQfdgM4QghRzr9fs8BFJxNUhsf9pfaNISvxCSFedgCHEEyXiOJzQH2xQD/HX580wdVJixdoHoaAOmBdpzTZBVTX0q7DTA8bOiLqR0WZ/fkm6tpLmALYUkDKd0Yy1aj3TgKK2Nseb/hn62fhLYYgJLOZAyICzfTd048WsiNvM+yD9kbsgrM6bvFhgbW9LNeLhg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=tevKfWGkHpgX117fvDDTrEgj66UjPgv7fNsmK7+ZLVA=; b=XLqnjDb/4tfPddtf00gGlFogNjDh+aUXNoazpIeVgauu8NdN3Gs57n1F/4JlKSCpmrjfQykOmoHKm3CDPyeZOKAxYxKQunlkeVZdGnWaJXUjziyXf7D0b+1wI5lYHOLmyz1F4TNuPw6IK0YaiJsQktSFRkWJ9083OGAuiG4GZHRdWVrSCoaJQNpmLauUyMaMV1WqgHXXU20u+CmoX5URKjmBmhxGR4ugreU31Xr0gNO6tyMnLoclNuNmTQgt+T96pt/anVK9rPHFZzTwEJhoXRYJUFLAJmrxluWnxl9SKhANBhYqEaThBqZZSoIGLxNL5GzEhrRCiJANjqmuIQZIMg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=mellanox.com; dmarc=pass action=none header.from=mellanox.com; dkim=pass header.d=mellanox.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=tevKfWGkHpgX117fvDDTrEgj66UjPgv7fNsmK7+ZLVA=; b=cJSf1New4EZJ4dE97jj05NC7AEKgX2zOd3XX7cFISKmiDnoL/v0b692yOl3/bXnYchDEFZ8vPqz9OnD/Ojs9N9iIQYnREXUz1SgWsEZYxUP5BMseKWp1dBvR6R3FaYx/JaiO6D1JprXpNv9Xn+g79uxLf0uYudTfbHKvkgf3A4E= Received: from HE1PR05MB3484.eurprd05.prod.outlook.com (2603:10a6:7:2f::12) by HE1PR05MB4668.eurprd05.prod.outlook.com (2603:10a6:7:98::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.20; Fri, 10 Apr 2020 10:34:14 +0000 Received: from HE1PR05MB3484.eurprd05.prod.outlook.com ([fe80::3022:cbe2:4523:718a]) by HE1PR05MB3484.eurprd05.prod.outlook.com ([fe80::3022:cbe2:4523:718a%4]) with mapi id 15.20.2900.015; Fri, 10 Apr 2020 10:34:14 +0000 From: Suanming Mou To: "Dumitrescu, Cristian" CC: "dev@dpdk.org" , "amo@semihalf.com" Thread-Topic: [PATCH v2 1/2] bitmap: add create bitmap with all bits set Thread-Index: AQHWDVLCGAx7wbO030GjaH+EmFcxhqhw1RVQgAEqGgA= Date: Fri, 10 Apr 2020 10:34:14 +0000 Message-ID: References: <1583828479-204084-1-git-send-email-suanmingm@mellanox.com> <1586315145-6633-1-git-send-email-suanmingm@mellanox.com> <1586315145-6633-2-git-send-email-suanmingm@mellanox.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=suanmingm@mellanox.com; x-originating-ip: [218.74.53.224] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: cee310cc-364e-43ca-d25c-08d7dd3ab73a x-ms-traffictypediagnostic: HE1PR05MB4668: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8273; x-forefront-prvs: 0369E8196C x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:HE1PR05MB3484.eurprd05.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(136003)(366004)(396003)(39860400002)(346002)(376002)(71200400001)(478600001)(33656002)(54906003)(9686003)(66946007)(5660300002)(66476007)(76116006)(26005)(86362001)(66446008)(64756008)(66556008)(4326008)(186003)(81156014)(55016002)(7696005)(316002)(2906002)(52536014)(53546011)(8936002)(6916009)(8676002)(6506007); DIR:OUT; SFP:1101; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: FdL1gU94C+1lrlzu5Vz5NHzqm8X5K83GejGDVKH8+vYEg2g32u5/iDWa7zmng9v/vxi8sx/gDSg2CDZqqcnWnRy53Uj9PL3HhKImGxFduTVWibUvTdgrMKEgjcZ2oakAz8uXuvaUIVcRjl2KdeJNNsm+NlFQT1s0lTpIOCl/luowLlVwyl3NrFEcarUdLlfS8RtHXBpP9z2UraDFMdRYGiVnK1AC+aVEZbrXrESNenLWSocotPGZv9wtjrZB18c3JcY9Ueu9zVkTKgR09d7umJFlO7wkjoZiNleHGE6mbJ6W94t6hUDo7DsRLlri58jmfSp107EZxiDnhYy/aLvo7WDs59r3ISF5EQQ14n22z3O2ouIjcME4MStzHmxsauj2m3w/aOFtvGZ93jY6bIbjrfJfV2JiPzAW6FDgArVplh9hSg1WwtDZiri1TrykSdU1 x-ms-exchange-antispam-messagedata: qTbpHt50u+nqRXw36UZUA0cl7ujLIFda+CvCoshhMJVxONlmAmFb46DAaJFxOvwtocpVNmOL5opL6NdNEYkJyN/PKVnReuSKyOLU2tVw4swdjrYM55YlqgCxJXdSNFLGeKLDZ4gfhqKd/kFJCeTYjA== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: cee310cc-364e-43ca-d25c-08d7dd3ab73a X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Apr 2020 10:34:14.7402 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: y4jPRxQjKy3HOQ72AVW1Ur770hTvQmHji1Xsf30zlEnx9BVeZpQ/n1wEWE0mtsRE8S5olFnEl7DiwjR3XQXDTA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR05MB4668 Subject: Re: [dpdk-dev] [PATCH v2 1/2] bitmap: add create bitmap with all bits set 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" > -----Original Message----- > From: Dumitrescu, Cristian > Sent: Thursday, April 9, 2020 10:16 PM > To: Suanming Mou > Cc: dev@dpdk.org; amo@semihalf.com > Subject: RE: [PATCH v2 1/2] bitmap: add create bitmap with all bits set >=20 > Hi Sunaming, >=20 > > -----Original Message----- > > From: Suanming Mou > > Sent: Wednesday, April 8, 2020 4:06 AM > > To: Dumitrescu, Cristian > > Cc: dev@dpdk.org; amo@semihalf.com > > Subject: [PATCH v2 1/2] bitmap: add create bitmap with all bits set > > > > Currently, in the case to use bitmap as resource allocator, after > > bitmap creation, all the bitmap bits should be set to indicate the bit > > available. Every time when allocate one bit, search for the set bits > > and clear it to make it in use. > > > > Add a new rte_bitmap_init_with_all_set() function to have a quick fill > > up the bitmap bits. > > > > Comparing with the case create the bitmap as empty and set the bitmap > > one by one, the new function costs less cycles. > > > > Signed-off-by: Suanming Mou > > --- > > lib/librte_eal/common/include/rte_bitmap.h | 113 > > ++++++++++++++++++++++------- > > 1 file changed, 85 insertions(+), 28 deletions(-) > > > > diff --git a/lib/librte_eal/common/include/rte_bitmap.h > > b/lib/librte_eal/common/include/rte_bitmap.h > > index 6b846f2..740076b 100644 > > --- a/lib/librte_eal/common/include/rte_bitmap.h > > +++ b/lib/librte_eal/common/include/rte_bitmap.h > > @@ -136,6 +136,40 @@ struct rte_bitmap { > > bmp->go2 =3D 0; > > } > > > > +static inline struct rte_bitmap * > > +__rte_bitmap_init(uint32_t n_bits, uint8_t *mem, uint32_t mem_size) { > > + struct rte_bitmap *bmp; > > + uint32_t array1_byte_offset, array1_slabs; > > + uint32_t array2_byte_offset, array2_slabs; > > + uint32_t size; > > + > > + /* Check input arguments */ > > + if (n_bits =3D=3D 0) > > + return NULL; > > + > > + if ((mem =3D=3D NULL) || (((uintptr_t) mem) & > > RTE_CACHE_LINE_MASK)) > > + return NULL; > > + > > + size =3D __rte_bitmap_get_memory_footprint(n_bits, > > + &array1_byte_offset, &array1_slabs, > > + &array2_byte_offset, &array2_slabs); > > + if (size < mem_size) > > + return NULL; > > + > > + /* Setup bitmap */ > > + bmp =3D (struct rte_bitmap *) mem; > > + > > + bmp->array1 =3D (uint64_t *) &mem[array1_byte_offset]; > > + bmp->array1_size =3D array1_slabs; > > + bmp->array2 =3D (uint64_t *) &mem[array2_byte_offset]; > > + bmp->array2_size =3D array2_slabs; > > + > > + __rte_bitmap_scan_init(bmp); > > + > > + return bmp; > > +} > > + > > /** > > * Bitmap memory footprint calculation > > * > > @@ -170,36 +204,12 @@ struct rte_bitmap { rte_bitmap_init(uint32_t > > n_bits, uint8_t *mem, uint32_t mem_size) { > > struct rte_bitmap *bmp; > > - uint32_t array1_byte_offset, array1_slabs, array2_byte_offset, > > array2_slabs; > > - uint32_t size; > > > > - /* Check input arguments */ > > - if (n_bits =3D=3D 0) { > > - return NULL; > > - } > > - > > - if ((mem =3D=3D NULL) || (((uintptr_t) mem) & > > RTE_CACHE_LINE_MASK)) { > > - return NULL; > > - } > > - > > - size =3D __rte_bitmap_get_memory_footprint(n_bits, > > - &array1_byte_offset, &array1_slabs, > > - &array2_byte_offset, &array2_slabs); > > - if (size < mem_size) { > > + bmp =3D __rte_bitmap_init(n_bits, mem, mem_size); > > + if (!bmp) > > return NULL; > > - } > > - > > - /* Setup bitmap */ > > - memset(mem, 0, size); > > - bmp =3D (struct rte_bitmap *) mem; > > - > > - bmp->array1 =3D (uint64_t *) &mem[array1_byte_offset]; > > - bmp->array1_size =3D array1_slabs; > > - bmp->array2 =3D (uint64_t *) &mem[array2_byte_offset]; > > - bmp->array2_size =3D array2_slabs; > > - > > - __rte_bitmap_scan_init(bmp); > > - > > + memset(bmp->array1, 0, bmp->array1_size * sizeof(uint64_t)); > > + memset(bmp->array2, 0, bmp->array2_size * sizeof(uint64_t)); > > return bmp; > > } > > >=20 > Can we please leave the function rte_bitmap_init() unmodified and put all > changes in the new function rte_bitmap_init_with_all_set(). I realize thi= s means > duplicating a few lines of code between the two init functions, but IMO e= asier to > maintain going forward. Sure. Agree with that, so let's keep the rte_bitmap_init() unmodified. >=20 > > @@ -483,6 +493,53 @@ struct rte_bitmap { > > return 0; > > } > > > > +/** > > + * Bitmap initialization with all bits set > > + * > > + * @param n_bits > > + * Number of pre-allocated bits in array2. > > + * @param mem > > + * Base address of array1 and array2. > > + * @param mem_size > > + * Minimum expected size of bitmap. > > + * @return > > + * Handle to bitmap instance. > > + */ > > +static inline struct rte_bitmap * > > +rte_bitmap_init_with_all_set(uint32_t n_bits, uint8_t *mem, uint32_t > > mem_size) > > +{ > > + uint32_t i; > > + uint32_t slabs, array1_bits; > > + struct rte_bitmap *bmp; > > + > > + bmp =3D __rte_bitmap_init(n_bits, mem, mem_size); > > + if (!bmp) > > + return NULL; > > + > > + array1_bits =3D bmp->array2_size >> > > RTE_BITMAP_CL_SLAB_SIZE_LOG2; > > + /* Fill the arry1 slab aligned bits. */ > > + slabs =3D array1_bits >> RTE_BITMAP_SLAB_BIT_SIZE_LOG2; > > + memset(bmp->array1, 0xff, slabs * sizeof(bmp->array1[0])); > > + /* Clear the array1 left slabs. */ > > + memset(&bmp->array1[slabs], 0, (bmp->array1_size - slabs) * > > + sizeof(bmp->array1[0])); > > + /* Fill the array1 middle not full set slab. */ > > + for (i =3D 0; i < (array1_bits & RTE_BITMAP_SLAB_BIT_MASK); i++) > > + bmp->array1[slabs] |=3D 1llu << i; > > + > > + /* Fill the arry2 slab aligned bits. */ > > + slabs =3D n_bits >> RTE_BITMAP_SLAB_BIT_SIZE_LOG2; > > + memset(bmp->array2, 0xff, slabs * sizeof(bmp->array2[0])); > > + /* Clear the array2 left slabs. */ > > + memset(&bmp->array2[slabs], 0, (bmp->array2_size - slabs) * > > + sizeof(bmp->array2[0])); > > + /* Fill the array2 middle not full set slab. */ > > + for (i =3D 0; i < (n_bits & RTE_BITMAP_SLAB_BIT_MASK); i++) > > + bmp->array2[slabs] |=3D 1llu << i; > > + > > + return bmp; > > +} > > + > > #ifdef __cplusplus > > } > > #endif > > -- > > 1.8.3.1 >=20 > This code is not that easy to read. This function is tricky to implement,= as we > basically need to correct some overhead bits in array1 and array2. >=20 > What I suggest for the layout of this function: > -call essentially the same code as rte_bitmap_init(), with the change tha= t we set > ALL the bits in array1 and array2 to 1 instead of 0 -call a new helper fu= nction to > correct (set to 0) all the array2 bits from position (index2, offset2) t= o the end - > call a new helper function to correct (set to 0) all the array1 bits from= position > (index1, offset1) to the end Good suggestion.=20 What about the function below, it will help both arry1 and array2 clear the= not needed bits: /** * Bitmap clear slab overhead bits. * * @param slab * Slab arrary. * @param size * Slab array size. * @param pos * The start bit position in the slabs to be cleared. */ static inline void __rte_bitmap_clear_slab_overhead_bits(uint64_t *slabs, uint32_t slab_size, uint32_t pos) { uint32_t i; uint32_t index =3D pos / RTE_BITMAP_SLAB_BIT_SIZE; uint32_t offset =3D pos & RTE_BITMAP_SLAB_BIT_MASK; if (offset) { for (i =3D offset; i < RTE_BITMAP_SLAB_BIT_SIZE; i++) slabs[index] &=3D ~(1llu << i); index++; } if (index < slab_size) memset(&slabs[index], 0, sizeof(slabs[0]) * (slab_size - index)); }=20 It seems that is a bit difficult to find a none tricky way to clear the bit= s. >=20 > What do you think? >=20 > Thanks, > Cristian