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 D156141D63; Mon, 27 Feb 2023 17:13:11 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B368640A84; Mon, 27 Feb 2023 17:13:11 +0100 (CET) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by mails.dpdk.org (Postfix) with ESMTP id EBDCF40A7D for ; Mon, 27 Feb 2023 17:13:10 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 9672F5C011B; Mon, 27 Feb 2023 11:13:10 -0500 (EST) Received: from imap48 ([10.202.2.98]) by compute1.internal (MEProxy); Mon, 27 Feb 2023 11:13:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=u256.net; h=cc :cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm2; t=1677514390; x= 1677600790; bh=uGMcaPAJMzK4UAmZzswg3HCrWbUfGRVdQtbFy+77iVw=; b=v tuG2icNQkpTmBoNTKncwQSG2lghdQw1txxYKs82p7fv+utFLQ+KFZHtPZlCJettQ tn1YBd5/aGNwWO6MV7p/cwnJGAQQNDnujlKuGKgqzcOtW2M50wbA+cX5+aYaQRZY oisWgtzd8uawcU7LA9eF7Cq6MaikrCst2CvZHHe8I8/3EpP1TC7ZBYD2dRSZ7XE1 oAJ6HP2IxuXUXowdf6fbzR9LqFNXDQhXOGXvkoAOkGORzihAq18Q77QRRCYL43v5 32WQvEvLspg0Fca1smsSCnbCiRWbmbqt/cm3U7lEYtpjq3VBlc255KnCRnE/eeLb JEIweB5HxKTChaIBMzcQg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1677514390; x= 1677600790; bh=uGMcaPAJMzK4UAmZzswg3HCrWbUfGRVdQtbFy+77iVw=; b=R sXbg+8qIcuW3w/BovrMPUNd1hxPX7kJ9dwwG2bmqUZwpvztQaJnKQ+0//M0c/Uic dJsW7m6a7bXyIG6k43JNH+p84thmbGqX5VpRWBteLWcTWRp244bXXnGOM5mLeFSn L6iowi40wrf1y3SKOMuBEbuEZgFm1euayA0ubzYYc5kmmbU3QwalnNFE2h5ZxwL2 z/aGVRupaZv9gBjL8iMz9kZMVviJ/CXvWM/fcI8iJn7+4tWRDH0adDDIzkBg73IW LLfXkYURNKojL4CLcDdJ6avRlnJumIb75OTI1tvmfApsEcJ4uWGpDouC6qNABb0z q151qLvKAWJpb4tdsAZZw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrudeltddgkedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvfevufgtgfesthhqredtreerjeenucfhrhhomhepifgr tohtrghnucftihhvvghtuceoghhrihhvvgesuhdvheeirdhnvghtqeenucggtffrrghtth gvrhhnpeefvdduhfdukeekjefgkeehiedukeefteeftdekvdevvefhtdeuveeuvdeufedu heenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehgrh hivhgvsehuvdehiedrnhgvth X-ME-Proxy: Feedback-ID: ib851476b:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id F3B8331A0064; Mon, 27 Feb 2023 11:13:09 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-183-gbf7d00f500-fm-20230220.001-gbf7d00f5 Mime-Version: 1.0 Message-Id: In-Reply-To: References: <20230224081642.2566619-1-david.marchand@redhat.com> <20230224151143.3274897-1-david.marchand@redhat.com> <095511d4-4fbb-47a8-a8a5-f2a3b932c01f@app.fastmail.com> Date: Mon, 27 Feb 2023 17:12:49 +0100 From: =?UTF-8?Q?Ga=C3=ABtan_Rivet?= To: "David Marchand" Cc: dev@dpdk.org, "Thomas Monjalon" Subject: Re: [PATCH v2 00/20] Enable lock annotations on most libraries and drivers 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 Sat, Feb 25, 2023, at 11:16, David Marchand wrote: > Salut Ga=C3=ABtan, > > On Fri, Feb 24, 2023 at 4:59 PM Ga=C3=ABtan Rivet wro= te: >> > FreeBSD libc pthread API has lock annotations while Linux glibc has >> > none. >> > We could simply disable the check on FreeBSD, but having this check, >> > a few issues got raised in drivers that are built with FreeBSD. >> > For now, I went with a simple #ifdef FreeBSD for pthread mutex rela= ted >> > annotations in our code. >> > >> >> Hi David, >> >> This is a great change, thanks for doing it. >> However I am not sure I understand the logic regarding the '#ifdef FR= EEBSD'. >> >> These annotations provide static hints to clang's thread safety analy= sis. >> What is the dependency on FreeBSD glibc? > > FreeBSD libc added clang annotations, while glibc did not. > > FreeBSD 13.1 libc: > int pthread_mutex_lock(pthread_mutex_t * __mutex) > __locks_exclusive(*__mutex); > > With: > > #if __has_extension(c_thread_safety_attributes) > #define __lock_annotate(x) __attribute__((x)) > #else > #define __lock_annotate(x) > #endif > > /* Function acquires an exclusive or shared lock. */ > #define __locks_exclusive(...) \ > __lock_annotate(exclusive_lock_function(__VA_ARGS__)) > > > glibc: > extern int pthread_mutex_lock (pthread_mutex_t *__mutex) > __THROWNL __nonnull ((1)); > > > > Since glibc does not declare that pthread_mutex_t is lockable, adding > an annotation triggers an error for Linux (taking eal_common_proc.c > patch 14 as an example): > > ../lib/eal/common/eal_common_proc.c:911:2: error: > 'exclusive_locks_required' attribute requires arguments whose type is > annotated with 'capability' attribute; type here is 'pthread_mutex_t' > [-Werror,-Wthread-safety-attributes] > __rte_exclusive_locks_required(pending_requests.lock) > ^ > ../lib/eal/include/rte_lock_annotations.h:23:17: note: expanded from > macro '__rte_exclusive_locks_required' > __attribute__((exclusive_locks_required(__VA_ARGS__))) > ^ > > We could wrap this annotation in a new construct (which would require > some build time check wrt pthread API state). > Not sure it is worth the effort though, for now. > > > --=20 > David Marchand Ah ok, so if I understand correctly, DPDK would need to declare some '__rte_lockable rte_mutex' and associated functions for transparent supp= ort, to wrap above the pthread API. Unless it happens, would it be possible to condition the thread-safety a= nnotations to FREEBSD as well? Maybe have two versions of the annotations: __rte_exclusive_locks_required() /* Conditioned on clang */ __pthread_exclusive_locks_required() /* Conditioned on glibc/pthread s= upport */ the first one to use on RTE types that can be declared with __rte_lockab= le, the second for pthread objects that we don't want to replace. I know the namespace is not great so maybe named another way. The '#ifdef FREEBSD' ossifies the annotation support at the function lev= el, when this is a matter of types. This impedance mismatch would mean large changes if someone was to make this support evolve at some point. --=20 Gaetan Rivet