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 1265B431FC; Wed, 25 Oct 2023 18:04:02 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C02C9402B3; Wed, 25 Oct 2023 18:04:01 +0200 (CEST) Received: from mail-pg1-f175.google.com (mail-pg1-f175.google.com [209.85.215.175]) by mails.dpdk.org (Postfix) with ESMTP id 441CF40273 for ; Wed, 25 Oct 2023 18:04:01 +0200 (CEST) Received: by mail-pg1-f175.google.com with SMTP id 41be03b00d2f7-517ab9a4a13so4646567a12.1 for ; Wed, 25 Oct 2023 09:04:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1698249840; x=1698854640; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=PvXxj2vS9etU4LTXBtKL93zIHb+dWRlNSYEfqTmEBfA=; b=PqUNPp+6azi1+LNxRjMwZu9hCm6qOJLiyJaZP8n9mBvdtkggqIfjarZ3yqRhQKRcIW k6xjxLMPhB7QrUWF4jqundDYuRm5unckcZg23syUHcH50HeAIQ3w+o0HgsSwKYv09kbV 2HZ+eSX/yFjJ32AVjkfoOxdOhfYVcVQfYRH4c65Dg6GnkFgu0gsbsi10JxwuHx2pSk1o v3KXTX6psNV6oXcL/fcNa46bBQnVrHMoVXRH894O5/53vGppM5dAjs0CDEFenaDGdSJa wsnirzrA5vfoIL+imxLfoY3ZiuKwBawmpkHUEtcxThGbVhHNmyoQxvjn3jFWj1YAUfWb +Wmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698249840; x=1698854640; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PvXxj2vS9etU4LTXBtKL93zIHb+dWRlNSYEfqTmEBfA=; b=nJyqgLp1o5PKLgvc8HoVhlsw6JsMoA32H0LHSW4RyzchrAbeTauLRAMeD3D3+uhyqr FThIGtxnPtyj8ihTYLe5jLozyoCJsnehYnJMHIgZvPEx5/VPxJ296XJQvsWoOgNjjQro +e5Hqw7Lw8hOqQ+NXNMre2iPQxDTER2jmV9gA3uSCwEfXFQ8fA81jUZpY83Bn2OxdXpz UvJfvj1tXExxQv5+cvJ1fAx6UsWa6QKdgqSdPJHXIhOpdkMlcCX+5VgD6bz1YWz7xEv3 IKpneYs1HlhuC8tW2nMiU87ULlXI7W0MEr26+TnPZEHqSt4rf3K4a/DUAyuRPqheWvo1 HVqw== X-Gm-Message-State: AOJu0Yzv2qJynDLKhzR3yAGzfBNedn4yq9ukV5TDn/zA6kGdv4en5E0+ HeKPhStGL26xJRUreT/9xNRSFg== X-Google-Smtp-Source: AGHT+IGashC6PlETe0yHJ7J80Diuji5stiaR6axG9+OFPRDP0Df+BINbmtXVpG+mAny4kE4b8XxNuQ== X-Received: by 2002:a17:90a:f30d:b0:27d:427f:1bf4 with SMTP id ca13-20020a17090af30d00b0027d427f1bf4mr14314010pjb.19.1698249839825; Wed, 25 Oct 2023 09:03:59 -0700 (PDT) Received: from hermes.local (204-195-126-68.wavecable.com. [204.195.126.68]) by smtp.gmail.com with ESMTPSA id 10-20020a17090a1a0a00b0027909a8994fsm86943pjk.13.2023.10.25.09.03.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Oct 2023 09:03:59 -0700 (PDT) Date: Wed, 25 Oct 2023 09:03:57 -0700 From: Stephen Hemminger To: Fengnan Chang Cc: Dmitry Kozlyuk , anatoly.burakov@intel.com, dev@dpdk.org, xuemingl@mellanox.com Subject: Re: [External] Re: [PATCH] eal: fix modify data area after memset Message-ID: <20231025090357.60c1f56e@hermes.local> In-Reply-To: References: <20230912090415.48709-1-changfengnan@bytedance.com> <20231022232234.42168129@sovereign> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 Mon, 23 Oct 2023 17:07:21 +0800 Fengnan Chang wrote: > Dmitry Kozlyuk =E4=BA=8E2023=E5=B9=B410=E6=9C= =8823=E6=97=A5=E5=91=A8=E4=B8=80 04:22=E5=86=99=E9=81=93=EF=BC=9A > > > > 2023-09-22 16:12 (UTC+0800), Fengnan Chang: =20 > > > ping > > > > > > Fengnan Chang =E4=BA=8E2023=E5=B9=B49=E6= =9C=8812=E6=97=A5=E5=91=A8=E4=BA=8C 17:05=E5=86=99=E9=81=93=EF=BC=9A =20 > > > > > > > > Let's look at this path: > > > > malloc_elem_free =20 > > > > ->malloc_elem_join_adjacent_free > > > > ->join_elem(elem, elem->next) =20 > > > > > > > > 0. cur elem's pad > 0 > > > > 1. data area memset in malloc_elem_free first. > > > > 2. next elem is free, try to join cur elem and next. > > > > 3. in join_elem, try to modify inner->size, this address had > > > > memset in step 1, it casue the content of addrees become non-zero. > > > > > > > > If user call rte_zmalloc, and pick this elem, it can't get all > > > > zero'd memory. =20 > > > > malloc_elem_join_adjacent_free() always calls memset() after join_elem(= ), > > for the next and the previous element respectively. =20 > when try to call join_elem() for the next element in > malloc_elem_join_adjacent_free(), > the memset is try to memset *next* element, but join_elem() is update > *current* element's > content, which shoudn't happen, it's two different element. >=20 > > How to reproduce this bug? =20 > when I test this patch, > https://patches.dpdk.org/project/dpdk/patch/20230831111937.60975-1-changf= engnan@bytedance.com/ > I have a case try to alloc 64/128/192 size object and free with 16 thread= s, > after every > alloc I'll check wheather all content is 0 or not. > It's not easy to reproduce, you can have a try, it's easier to find > this problem in code level. I tried to make a test that would reproduce the problem but it did not. diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c index cd579c503cf5..cfd45d6a28eb 100644 --- a/app/test/test_malloc.c +++ b/app/test/test_malloc.c @@ -28,6 +28,7 @@ #include #define N 10000 +#define BINS 100 static int is_mem_on_socket(int32_t socket); @@ -69,13 +70,24 @@ is_aligned(void *p, int align) return 1; } +static bool is_all_zero(uint8_t *mem, size_t sz) +{ + size_t i; + + for (i =3D 0; i < sz; i++) + if (mem[i] !=3D 0) + return false; + + return true; +} + static int test_align_overlap_per_lcore(__rte_unused void *arg) { const unsigned align1 =3D 8, align2 =3D 64, align3 =3D 2048; - unsigned i,j; + unsigned int i; void *p1 =3D NULL, *p2 =3D NULL, *p3 =3D NULL; int ret =3D 0; @@ -86,11 +98,12 @@ test_align_overlap_per_lcore(__rte_unused void *arg) ret =3D -1; break; } - for(j =3D 0; j < 1000 ; j++) { - if( *(char *)p1 !=3D 0) { - printf("rte_zmalloc didn't zero the allocated memory\n"); - ret =3D -1; - } + + if (!is_all_zero(p1, 1000)) { + printf("rte_zmalloc didn't zero the allocated memory\n"); + ret =3D -1; + rte_free(p1); + break; } p2 =3D rte_malloc("dummy", 1000, align2); if (!p2){ @@ -140,6 +153,66 @@ test_align_overlap_per_lcore(__rte_unused void *arg) return ret; } +/* + * Allocate random size chunks and make sure that they are + * always zero. + */ +static int +test_zmalloc(__rte_unused void *arg) +{ + unsigned int i, n; + void *slots[BINS] =3D { }; + void *p1; + size_t sz; + + /* Allocate many variable size chunks */ + for (i =3D 0; i < BINS; i++) { + sz =3D rte_rand_max(1024) + 1; + p1 =3D rte_zmalloc("slots", sz, 0); + if (p1 =3D=3D NULL) { + printf("rte_zmalloc(%zu) returned NULL (i=3D%u)\n", sz, i); + goto fail; + } + slots[i] =3D p1; + if (!is_all_zero(p1, sz)) + goto fail; + } + + /* Drop one chunk per iteration */ + for (n =3D BINS; n > 0; n--) { + /* Swap in a new block into a slot */ + for (i =3D 0; i < N; i++) { + unsigned int bin =3D rte_rand_max(n); + + sz =3D rte_rand_max(1024) + 1; + p1 =3D rte_zmalloc("swap", sz, 0); + if (!p1){ + printf("rte_zmalloc(%zu) returned NULL (i=3D%u)\n", sz, i); + goto fail; + } + + if (!is_all_zero(p1, sz)) { + printf("rte_zmalloc didn't zero the allocated memory\n"); + goto fail; + } + + rte_free(slots[bin]); + slots[bin] =3D p1; + } + + /* Drop last bin */ + rte_free(slots[n]); + slots[n] =3D NULL; + } + + return 0; +fail: + for (i =3D 0; i < BINS; i++) + rte_free(slots[i]); + + return -1; +} + static int test_reordered_free_per_lcore(__rte_unused void *arg) { @@ -1020,6 +1091,21 @@ test_malloc(void) } else printf("test_realloc() passed\n"); + /*----------------------------*/ + RTE_LCORE_FOREACH_WORKER(lcore_id) { + rte_eal_remote_launch(test_zmalloc, NULL, lcore_id); + } + + RTE_LCORE_FOREACH_WORKER(lcore_id) { + if (rte_eal_wait_lcore(lcore_id) < 0) + ret =3D -1; + } + if (ret < 0){ + printf("test_zmalloc() failed\n"); + return ret; + } + else printf("test_zmalloc() passed\n"); + /*----------------------------*/ RTE_LCORE_FOREACH_WORKER(lcore_id) { rte_eal_remote_launch(test_align_overlap_per_lcore, NULL, lcore_id);