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 1AD6741C23; Mon, 6 Feb 2023 17:11:35 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EBF3B42B7E; Mon, 6 Feb 2023 17:11:34 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id BE5ED427E9 for ; Mon, 6 Feb 2023 17:11:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675699893; 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=hs96BQHcAkeEHhZ6Ob/utefQGOrj1DDtAphvtfUs1ss=; b=P8+sPIQ8J+DPNTy5gnv7/EYmDPZ8C4uYV90zF8zzBSJRy/JMRpf2Lq5yUk2ZGgMG/eYZpV gNt5tPK8huQqftwQWI5OM0ljNzZKyORZ7zlFoL7FWUQvkmInff9hjWcPZvygBVT2nw1r7J uNK2MwOr/v/jRpVzCwk4yxhIfFaN9xs= Received: from mail-pg1-f197.google.com (mail-pg1-f197.google.com [209.85.215.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-505--cW7yv8LO8e5ke7xiKxoPg-1; Mon, 06 Feb 2023 11:11:32 -0500 X-MC-Unique: -cW7yv8LO8e5ke7xiKxoPg-1 Received: by mail-pg1-f197.google.com with SMTP id 201-20020a6300d2000000b004ccf545f44fso5348034pga.12 for ; Mon, 06 Feb 2023 08:11:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=hs96BQHcAkeEHhZ6Ob/utefQGOrj1DDtAphvtfUs1ss=; b=BlQ/aNvLavVnENQM/OQPEZ9lxbX1MmXSqi4xUrkw/byh5PPhOUQPH3+DUzhDDt/w61 eP6tUKKeoyOWU7uq40wOv8HxV4N7BDpH+s4wTMXHwV7ZjOVhwXyRHcg6BDXLfXw+RYiH 0lqBcHo/lhKJfLz1ikb5yQaT3SYX8pR1rb56hZbGLqcSm4keVn2NHrNx4lkksa879JfG LbZFaxrEbwSpMtB2XpMDZhJTvCaEe64Ur14wWvRNHrQxGH5zAz5T0wwtMX40yjv0Wn55 iq5sjo/1F2XBP4UAq8/cHgH7+9ehWoXwZPEGHz0D3RvxOKtYCwWwwdfTJUxp9weozUAI eDkA== X-Gm-Message-State: AO0yUKVyqSjrx8iSZSaGX2XJgc3xutCkh+Eqd6E9eC2jdTJbKAbquPAl 9X0Qi4ubbDabTwlPu0hMhnS62wsld2PYh1FPp1guOY1ltYj2bCfglIlvm3CX+Rim4UAzcRVcGFg jVC7L8nrGBv4dhBJT6dM= X-Received: by 2002:a62:384f:0:b0:592:6299:43d2 with SMTP id f76-20020a62384f000000b00592629943d2mr6532pfa.1.1675699891098; Mon, 06 Feb 2023 08:11:31 -0800 (PST) X-Google-Smtp-Source: AK7set95F5tEBlzMcF7KO2boWYioNKbRi1AYUI1Fv6UHtYpv5PVQEN9oJFQUyELh3yEk5Fw9qOY3pUuYUAWEHF+KXVw= X-Received: by 2002:a62:384f:0:b0:592:6299:43d2 with SMTP id f76-20020a62384f000000b00592629943d2mr6512pfa.1.1675699890811; Mon, 06 Feb 2023 08:11:30 -0800 (PST) MIME-Version: 1.0 References: <20221202153432.131023-1-mb@smartsharesystems.com> <3659208.MHq7AAxBmi@thomas> <98CBD80474FA8B44BF855DF32C47DC35D876E6@smartserver.smartshare.dk> <5353813.29KlJPOoH8@thomas> In-Reply-To: <5353813.29KlJPOoH8@thomas> From: David Marchand Date: Mon, 6 Feb 2023 17:11:19 +0100 Message-ID: Subject: Re: [PATCH v7 4/4] eal: add nonnull and access function attributes To: Thomas Monjalon Cc: Tyler Retzlaff , =?UTF-8?Q?Morten_Br=C3=B8rup?= , bruce.richardson@intel.com, Ferruh Yigit , dev@dpdk.org, rmody@marvell.com, timothy.mcdaniel@intel.com, matan@nvidia.com, viacheslavo@nvidia.com, ruifeng.wang@arm.com, zhoumin@loongson.cn, drc@linux.vnet.ibm.com, kda@semihalf.com, konstantin.v.ananyev@yandex.ru, stephen@networkplumber.org, jerinj@marvell.com, honnappa.nagarahalli@arm.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" 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 Wed, Feb 1, 2023 at 2:16 PM Thomas Monjalon wrote: > > > I tend to prefer this kind of namespace as well. > > > Let's compare different naming proposals, > > > taking into account what we already have for some annotations, > > > and what is proposed to be added in this patch and David's patch > > > for lock annotations. > > > > David's lock annotations don't use a dedicated naming convention, but simply replaces __attribute__(name(params)) with __rte_name(params), e.g.: > > > > #define __rte_guarded_by(...) \ > > __attribute__((guarded_by(__VA_ARGS__))) > > #define __rte_exclusive_locks_required(...) \ > > __attribute__((exclusive_locks_required(__VA_ARGS__))) > > #define __rte_assert_exclusive_lock(...) \ > > __attribute__((assert_exclusive_lock(__VA_ARGS__))) > > > > This follows the existing convention in rte_common.h, which is easily readable, because they directly translate to GCC attribute names, e.g.: > > > > #define __rte_warn_unused_result __attribute__((warn_unused_result)) > > #define __rte_always_inline inline __attribute__((always_inline)) > > #define __rte_noinline __attribute__((noinline)) > > #define __rte_hot __attribute__((hot)) > > #define __rte_cold __attribute__((cold)) > > > > I could follow this convention too, e.g.: > > > > #define __rte_nonnull(...) __attribute__((nonnull(__VA_ARGS__))) > > > > #define __rte_access_read_only(...) \ > > __attribute__((access(read_only, __VA_ARGS__))) > > #define __rte_access_write_only(...) \ > > __attribute__((access(write_only, __VA_ARGS__))) > > > [...] > > > Longer macro names may be better for code readers. > > > > You have a good point here, Thomas. It will also prevent using the access mode attribute macros incorrectly. > > > > Let's proceed with two macros (without and with size parameter) instead of the combined macros (with optional size parameter). > > > > Referring to the existing convention in rte_common.h, what do you think about naming the new macros as follows? > > > > __rte_access_read_only(ptr_index) > > __rte_access_read_only_size(ptr_index, size_index) > > > > Or just: > > > > __rte_read_only(ptr_index) > > __rte_read_only_size(ptr_index, size_index) > > I think we don't need the word "access", so I prefer the second form. > > What about the proposal of having "param" in the name? > We could also have "function" is some macro names. > Does it bring a benefit? > > Please let's have a naming discussion with many opinions. Thomas, I think naming convention/discussion is the most likely to never conclude. Just copying my previous suggestion. __rte_read_only_params(indexes...) __rte_write_only_params(indexes...) __rte_no_access_params(indexes...) So if we are not going with the existing (kind of) convention to just prefix with __rte_, I prefer Morten second form too and I have no better idea. As for the lock annotations series, if you are not confident with the form I went with, I don't mind deferring to a later release. Though it adds more work on my pile like rebasing the vhost library. Additionnally, we lose the opportunity to catch introduction of new lock issues in the dpdk tree. -- David Marchand