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 AF99845CF8; Wed, 13 Nov 2024 17:48:42 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 926DC40A67; Wed, 13 Nov 2024 17:48:42 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 0917E406BC for ; Wed, 13 Nov 2024 17:48:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1731516520; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vmZhfK8Thvd7aEertvNBvrR6V0fp+FBBN0UKsch4708=; b=H8mTGO2jS5BFOQXxCxNenZGksnEVT2oM0UU8wL/51hW+Rd8ZPR3faCHmZMVK0iHsSnCr39 5ohbKpnPnJjhJuBDULGf5jto/do43/Zv8MiZo1IOcEhcRNIevPZn7smaQOyNu/3feYja6k XqPi8FnBzn1+dXM+wf9JdrSSho3YlOs= Received: from mail-lj1-f199.google.com (mail-lj1-f199.google.com [209.85.208.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-546-dTWsig8hO4i-dpm3PIIiyg-1; Wed, 13 Nov 2024 11:48:37 -0500 X-MC-Unique: dTWsig8hO4i-dpm3PIIiyg-1 X-Mimecast-MFC-AGG-ID: dTWsig8hO4i-dpm3PIIiyg Received: by mail-lj1-f199.google.com with SMTP id 38308e7fff4ca-2fb44181f04so46450461fa.2 for ; Wed, 13 Nov 2024 08:48:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731516516; x=1732121316; h=content-transfer-encoding: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=vmZhfK8Thvd7aEertvNBvrR6V0fp+FBBN0UKsch4708=; b=hdhZF8OBBeBogsug7+EDKWfA30x+ipssVZ/V0ErqvYOqv9FFTfrYKt/RrNcPnjTsMe 02u/zFbNOz0PC7plS3vyDhL12dCeAQIMuj5i1Ask3nMEJvpvi05jW9RM6J+U0GoB04ri CGCcA5qzpFK9NVwm/IJq9apC8MuLr3qcXMl6W9adOz3MYXQIMTdZR9t10xjN3BeRtmAP Kg1gZQ1xz3QXxnHeTGAWuIPlwUYEmsLl3Iv/RScr68ksWlv0zfcxneDolat9/PWeyEOa tVlkZsATC0kSaALSYap8acZj42P38fmyn2IKQh+dAHyjsx9klldzev5Buht9OhtIWOk1 0Dlw== X-Forwarded-Encrypted: i=1; AJvYcCUnT2RRTE0Q5xYsJgEyBX8YBr6MlWFqiXXZADjVHgac6ZLM0C5nsYzS89TLnK3WagIG95g=@dpdk.org X-Gm-Message-State: AOJu0Yyr3mZfjmS/NNVMCRYnN4dfcliRqCzI1bc7lHyFrQo12G7pJ/vX qGDjxh+evFRCcmvAx2jOw5IrazhYtnYRImgQlcE0iP70umlcDr/WukUETRO0h5nA9cBJ2lnYbb8 tVD9/ecAK0I3Nw7GtKCH3Nu2IgrFMCPGY5o1BxVBj/e3tp45u6+nmvxpO6/HxBhBjUAfim7Z+WI LbPrE/qEiYqArG330= X-Received: by 2002:a05:651c:4118:b0:2ff:4ce0:d268 with SMTP id 38308e7fff4ca-2ff4ce0d4c0mr14324651fa.2.1731516516356; Wed, 13 Nov 2024 08:48:36 -0800 (PST) X-Google-Smtp-Source: AGHT+IFkTaztNB49uwdFwukXe4EyyD06At46Opllcl5ApqJ8iRn6DWIBrEi9m2B4IL1dSHb2W3pWllw7MFCU8G3rxHI= X-Received: by 2002:a05:651c:4118:b0:2ff:4ce0:d268 with SMTP id 38308e7fff4ca-2ff4ce0d4c0mr14324591fa.2.1731516515865; Wed, 13 Nov 2024 08:48:35 -0800 (PST) MIME-Version: 1.0 References: <20241113161455.2649551-1-hemant.agrawal@nxp.com> In-Reply-To: <20241113161455.2649551-1-hemant.agrawal@nxp.com> From: David Marchand Date: Wed, 13 Nov 2024 17:48:25 +0100 Message-ID: Subject: Re: [PATCH 1/2] bus/dpaa: fix lock condition during error handling To: Hemant Agrawal Cc: thomas@monjalon.net, dev@dpdk.org, stephen@networkplumber.org, stable@dpdk.org X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: F6Yg6qFBxewkKId_j_CykfmbM7o2LRR0SAq2r069iwc_1731516516 X-Mimecast-Originator: redhat.com 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 Hello Hemant, On Wed, Nov 13, 2024 at 5:15=E2=80=AFPM Hemant Agrawal wrote: > > The error handling is missing FQ unlock code. > Detected by pvr-studio > Bug 89-93: very suspicious synchronization > The analyzer issued a pack of V1020 warnings that a resource > might remain blocked. > > Fixes: c47ff048b99a ("bus/dpaa: add QMAN driver core routines") > Cc: stable@dpdk.org > > Signed-off-by: Hemant Agrawal The fix looks good to me. To avoid regressions on this topic, could you look into enabling clang thread safety check? I suppose that FQLOCK and FQUNLOCK always come in pairs (seems a safe assumption), so enabling should be something like: $ git diff --cached diff --git a/drivers/bus/dpaa/base/qbman/qman.c b/drivers/bus/dpaa/base/qbman/qman.c index 9c90ee25a6..48f181c004 100644 --- a/drivers/bus/dpaa/base/qbman/qman.c +++ b/drivers/bus/dpaa/base/qbman/qman.c @@ -27,18 +27,8 @@ * protection (and indeed, attempting to nest irq-protection doesn't work,= as * the "irq en/disable" machinery isn't recursive...). */ -#define FQLOCK(fq) \ - do { \ - struct qman_fq *__fq478 =3D (fq); \ - if (fq_isset(__fq478, QMAN_FQ_FLAG_LOCKED)) \ - spin_lock(&__fq478->fqlock); \ - } while (0) -#define FQUNLOCK(fq) \ - do { \ - struct qman_fq *__fq478 =3D (fq); \ - if (fq_isset(__fq478, QMAN_FQ_FLAG_LOCKED)) \ - spin_unlock(&__fq478->fqlock); \ - } while (0) +#define FQLOCK(fq) fq_lock(fq) +#define FQUNLOCK(fq) fq_unlock(fq) static qman_cb_free_mbuf qman_free_mbuf_cb; @@ -57,6 +47,22 @@ static inline int fq_isset(struct qman_fq *fq, u32 mask) return fq->flags & mask; } +static inline void fq_lock(struct qman_fq *fq) + __rte_exclusive_lock_function(&fq->fqlock) + __rte_no_thread_safety_analysis +{ + if (fq_isset(fq, QMAN_FQ_FLAG_LOCKED)) + spin_lock(&fq->fqlock); +} + +static inline void fq_unlock(struct qman_fq *fq) + __rte_unlock_function(&fq->fqlock) + __rte_no_thread_safety_analysis +{ + if (fq_isset(fq, QMAN_FQ_FLAG_LOCKED)) + spin_unlock(&fq->fqlock); +} + static inline int fq_isclear(struct qman_fq *fq, u32 mask) { return !(fq->flags & mask); diff --git a/drivers/bus/dpaa/meson.build b/drivers/bus/dpaa/meson.build index 114e0a2265..5506f2bffc 100644 --- a/drivers/bus/dpaa/meson.build +++ b/drivers/bus/dpaa/meson.build @@ -29,5 +29,3 @@ if cc.has_argument('-Wno-pointer-arith') endif includes +=3D include_directories('include', 'base/qbman') - -annotate_locks =3D false --=20 David Marchand