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 0A47CA059F; Fri, 10 Apr 2020 14:30:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DEE241D52A; Fri, 10 Apr 2020 14:30:27 +0200 (CEST) Received: from EUR04-DB3-obe.outbound.protection.outlook.com (mail-eopbgr60074.outbound.protection.outlook.com [40.107.6.74]) by dpdk.org (Postfix) with ESMTP id B73701D515 for ; Fri, 10 Apr 2020 14:30:26 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=HKlWiRC+phwLTH1RT4qXPu6cua3w3r8UuuY8se4ZpIM5lZdPU2ZevvGT6ycyT85TcdXqrjc94iIHZkCu6eJ8VOXTPd2FQtMwEIoPZ7T7fLzXf0WwSb3URYGoIylliGx6Ch6e6/UrmktQWdHbCZPTOnwFSskEiHecD1rdGHs6YoAYe+/UvHUEOrwQHhBYDaAeP8kL8FU/wHrtZ2MUmPIfeiLYTOIMxTyptZnDxHtiuH0J7WstcJvTOUGmThVzIem2pE+GGx/uAFauRGYMCnKri4fMrDyoHPLyISZEXAQB1+yBf8csoF8m38YPATBtOmqZn01ibIMcg/iKiv0aznVPQw== 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=q62YxoQI1klUqTqirY/VCHjv3S6StwNisFBF+1EnkGI=; b=NcmIDtW5z3ro6AoKI/y/g3gh3OwiWnzcuol6wkxWaE+Sbd0gdMMl1GRIEmhChmKzC5oZfUC7qLjBaAG9MiK08LLtc2+Ilgydb9LXQcEd1bqzD49HGH/eGayS9N8B/GLMeFWYBi79x671lDNd8YP5K1QqrC+tlGgGErYCqz3Mc7agJvw9mVEUY9GFwpL8K3eWljIir0/Ismu70zkf1l2GfR/z5jeFu071oPQYqaZ1JtvAUX3kWj3GAuoLBvhyaGpGT9ieX4CYASV2/R/a++16S4LFWg4FPDiDGy38Qds+ncNZo29GkQSlJT8NzSPvvi0ANC7iRBW3mcV0ToCRvcZTGA== 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=q62YxoQI1klUqTqirY/VCHjv3S6StwNisFBF+1EnkGI=; b=IGfY+j288rkDJPYMzJ0tD5DHbb8Yvele++OvLP9arZIquC1HOroiscuQdOB/6Xevu1O5nGtXWuFhrUGJLYK4pmSD4Qsbl00B7WoT2hhYddGDksdspE94g/A1ILfFZCd6znWF7KHo7H83OigJGSvBKCTzXQKnFLoHaIe8F/MualA= Received: from HE1PR05MB3484.eurprd05.prod.outlook.com (2603:10a6:7:2f::12) by HE1PR05MB3498.eurprd05.prod.outlook.com (2603:10a6:7:33::18) 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 12:30:20 +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 12:30:20 +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+EmFcxhqhw1RVQgAEqGgCAADmIUIAAE+Wg Date: Fri, 10 Apr 2020 12:30:20 +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: 2233194f-1707-4129-8189-08d7dd4aef1e x-ms-traffictypediagnostic: HE1PR05MB3498: 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)(366004)(346002)(136003)(376002)(396003)(39860400002)(54906003)(33656002)(86362001)(478600001)(66946007)(2906002)(52536014)(66476007)(66556008)(53546011)(186003)(7696005)(76116006)(66446008)(6506007)(26005)(6916009)(4326008)(64756008)(316002)(9686003)(81156014)(71200400001)(55016002)(8936002)(5660300002)(8676002); 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: kPDMJb/pPLuB3IB7JhQ0r1WOgsc07lHMrxbgFBzMAXqB+Wk0MXARKaky0L5w1mkG62gHj0C3JWal1LDioyCiSzJcqFBguPbDJ6rV5Sf0h5gXqXSYZTeq6omC1/VVARZkDImfTiHvSXrkGbe6EVIPikAPXju4VHyS1l6AwsoA+/ynib64wz7Pnsnw0AN0H1LMdoSgayBraKMaAMJY5KmCOrLnVUswWY5ZMNt3elGYH8kZ/z4/3QYcPPXH8l3rFJSOu0OYDTKGyyfOsF0Xmyw8eR8on6rK9++9opDEVWB85fwkv7c6/C1ABoKlnKepVYu3T5aL+3XHIQWmLNYA4nDwGSK+SXttsUJDSxlTukTbKL9kgSwtWqgYVzqKFTU++AbMWzaj5VH5giOVfQZcOm38nbsytIK+IQM1JwWRJq83C9u34QH/+o/FVU223JrTYYn/ x-ms-exchange-antispam-messagedata: /eTfoQOHESTegwAucqAfg9HDZNLHcEOFCQVRZG5UIN/quaAwOJNZb3v/ASYUflNb3LHDT1xA1OvBG0Ro+WW0WqZI0qO5mJcbAZ8zZbVaA351EjdwaG2MIG2dSJqbhpx0VF4xZ8+KN7y4UIFWXq6wRw== 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: 2233194f-1707-4129-8189-08d7dd4aef1e X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Apr 2020 12:30:20.4057 (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: buBcDyud6utqfhfsGpqEzJjwkEtJdOpQ3ZXbfHXMMyzZRxgkeassxitz1hmHJn1RrgyMammXb6WHLHmL0I4rZA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR05MB3498 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" Hi Cristian Thanks for the suggestion. Will update the v3 patch. BR SuanmingMou > -----Original Message----- > From: Dumitrescu, Cristian > Sent: Friday, April 10, 2020 7:21 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 >=20 >=20 > > -----Original Message----- > > From: Suanming Mou > > Sent: Friday, April 10, 2020 11:34 AM > > To: Dumitrescu, Cristian > > Cc: dev@dpdk.org; amo@semihalf.com > > Subject: RE: [PATCH v2 1/2] bitmap: add create bitmap with all bits > > set > > > > > > > > > -----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 > > > > > > Hi Sunaming, > > > > > > > -----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; > > > > } > > > > > > > > > > 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 this > > means > > > duplicating a few lines of code between the two init functions, but > > > IMO > > easier to > > > maintain going forward. > > > > Sure. Agree with that, so let's keep the rte_bitmap_init() unmodified. > > > > > > > @@ -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 > > > > > > 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. > > > > > > What I suggest for the layout of this function: > > > -call essentially the same code as rte_bitmap_init(), with the > > > change that > > we set > > > ALL the bits in array1 and array2 to 1 instead of 0 -call a new > > > helper function > > to > > > correct (set to 0) all the array2 bits from position (index2, > > > offset2) to 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. > > 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. >=20 > For more clarity, maybe document the size parameter as: number of 64-bit = slabs > in the slabs array. >=20 > > * @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_si= ze, > > 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 > Excellent, I like it, thanks Suanming! >=20 > > It seems that is a bit difficult to find a none tricky way to clear the= bits. > > > > > > What do you think? > > > > > > Thanks, > > > Cristian