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 6DD68A0524; Wed, 5 May 2021 22:28:50 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CC73840040; Wed, 5 May 2021 22:28:49 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mails.dpdk.org (Postfix) with ESMTP id 989384003C for ; Wed, 5 May 2021 22:28:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620246527; 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: in-reply-to:in-reply-to:references:references; bh=SMYj9D/3kXVNs9D8Q73CdGJVGbjc6BPyW9uYrh+qBSM=; b=Yp09BvH4sxXBsNBchCKR1GdKmQtzyB6xpbKsQGvQwQ2BI+rQlKUy4s1VHRFV8HTQtr4du+ yqDhdbT+2W9UcC3ZCMQnOorT+n15qd1ahqoCxDdchDyvvYk0Uq6nZazpcclBboko+vZ+Z3 Lrcxfx1UZm5spJUWfUwa+GBS4gn3u/U= Received: from mail-ua1-f71.google.com (mail-ua1-f71.google.com [209.85.222.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-557-AP_wmuxlM4eO5JLrSxf7Og-1; Wed, 05 May 2021 16:28:46 -0400 X-MC-Unique: AP_wmuxlM4eO5JLrSxf7Og-1 Received: by mail-ua1-f71.google.com with SMTP id g10-20020ab039ca0000b02901f7b6d6a473so228738uaw.17 for ; Wed, 05 May 2021 13:28:46 -0700 (PDT) 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=SMYj9D/3kXVNs9D8Q73CdGJVGbjc6BPyW9uYrh+qBSM=; b=sEes0TYxItpuL54SJInvW6xHpnSLskuHY8dDt5wMkvO4JOtk29g51SL7P0MKr4xj9K xyFCraR6wccTn2BxPt38nKRMPmqeeqpx8WjXP9TcyC7T2H9yWE/yEXtRyUhjDjqv/yap iamZC8njlVZYs1rK1mJGr+90VugzicCxd1CV/sLbkLyIbsYj8Ubml/ihorwY/GQG+yyJ WhESqlPafMKMEmZ5rp+WjyZN5Ni07C4AuudzYTgZMsIQgLAMARVkRaS9D0NqVRTLQqvN 7z2QTnBy1k1iPqpKCQ/9XN0DkiJr8hT88vez+29pXZ+LG3bTHz2v+zwGBb2NZLbFmyoS WO1w== X-Gm-Message-State: AOAM531qkqO3MYjExKsmJKBMPtZTbk08x66uosKU2LgL5Bs2+aKIa9lC Ev9fH6J4JDTM9GytmOUE+BiXxbU8Q5W/E1BXwAdF6Y5x6wy14ZnMvuHckc/Yrm1F/84KiG6S6o0 WuXQKbE0gTzppr55pqOE= X-Received: by 2002:a05:6122:1349:: with SMTP id f9mr389417vkp.18.1620246525667; Wed, 05 May 2021 13:28:45 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz5HAlW+2VC3SdvZCfpKWNEK1e06NQeD3EM+NrxiLqyRYbrsx0fgFBTD5bJ/nWBQf3JY1gGn2C9tykX6a4XB9U= X-Received: by 2002:a05:6122:1349:: with SMTP id f9mr389399vkp.18.1620246525398; Wed, 05 May 2021 13:28:45 -0700 (PDT) MIME-Version: 1.0 References: <1615496911-7050-1-git-send-email-roretzla@linux.microsoft.com> <20210505163034.GA27627@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> In-Reply-To: <20210505163034.GA27627@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: David Marchand Date: Wed, 5 May 2021 22:28:34 +0200 Message-ID: To: Tyler Retzlaff Cc: dev , Thomas Monjalon , Aaron Conole , Pavan Nikhilesh Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] eal: avoid side effects in RTE_ALIGN_MUL_NEAR(v, mul) for v and mul 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 Sender: "dev" On Wed, May 5, 2021 at 6:30 PM Tyler Retzlaff wrote: > > On Fri, Mar 12, 2021 at 09:07:22AM +0100, David Marchand wrote: > > On Thu, Mar 11, 2021 at 10:08 PM Tyler Retzlaff > > wrote: > > > > > > Avoid expanding v and mul parameters multiple times in the macro. based > > > on usage of the macro it seems like side effects were not intended. > > > > > > For example: > > > ``return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, CYC_PER_10MHZ);'' > > > > That's the beauty of macros. > > How about updating the unit tests so that this kind of issue is not > > reintroduced? > > i'm afraid i don't have the schedule budget to do this. i get very > little dpdk time. I started to write a test earlier (see below) but I did not finish either. We can wait until somebody has the time to investigate/finish the work. I copied Pavan who authored RTE_ALIGN_MUL_*. There are more macros affected than the one you fixed (see commented lines with FIXME:), and there are probably more macros to check in rte_common.h: diff --git a/app/test/test_common.c b/app/test/test_common.c index 12bd1cad90..44770722a7 100644 --- a/app/test/test_common.c +++ b/app/test/test_common.c @@ -18,6 +18,13 @@ {printf(x "() test failed!\n");\ return -1;} +static uintptr_t callcount; +static uintptr_t +inc_callcount(void) +{ + return callcount++; +} + /* this is really a sanity check */ static int test_macros(int __rte_unused unused_parm) @@ -28,8 +35,18 @@ test_macros(int __rte_unused unused_parm) #define FAIL_MACRO(x)\ {printf(#x "() test failed!\n");\ return -1;} +#define TEST_SIDE_EFFECT_2(macro, type1, type2) do { \ + callcount = 0; \ + (void)macro((type1)inc_callcount(), (type2)inc_callcount()); \ + if (callcount != 2) { \ + printf(#macro" has side effects: callcount=%u\n", \ + (unsigned int)callcount); \ + ret = -1; \ + } \ +} while (0) uintptr_t unused = 0; + int ret = 0; RTE_SET_USED(unused); @@ -47,7 +64,19 @@ test_macros(int __rte_unused unused_parm) if (strncmp(RTE_STR(test), "test", sizeof("test"))) FAIL_MACRO(RTE_STR); - return 0; + TEST_SIDE_EFFECT_2(RTE_PTR_ADD, void *, size_t); + TEST_SIDE_EFFECT_2(RTE_PTR_DIFF, void *, void *); + TEST_SIDE_EFFECT_2(RTE_PTR_SUB, void *, size_t); + /* FIXME: TEST_SIDE_EFFECT_2(RTE_PTR_ALIGN, void *, size_t); */ + /* FIXME: TEST_SIDE_EFFECT_2(RTE_PTR_ALIGN_CEIL, void *, size_t); */ + TEST_SIDE_EFFECT_2(RTE_PTR_ALIGN_FLOOR, void *, size_t); + /* FIXME: TEST_SIDE_EFFECT_2(RTE_ALIGN, unsigned int, unsigned int); */ + /* FIXME: TEST_SIDE_EFFECT_2(RTE_ALIGN_CEIL, unsigned int, unsigned int); */ + TEST_SIDE_EFFECT_2(RTE_ALIGN_FLOOR, unsigned int, unsigned int); + /* FIXME: TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_CEIL, unsigned int, unsigned int); */ + /* FIXME: TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_FLOOR, unsigned int, unsigned int); */ + /* FIXME: TEST_SIDE_EFFECT_2(RTE_ALIGN_MUL_NEAR, unsigned int, unsigned int); */ + return ret; } static int -- David Marchand