Hello Hemant,
On Wed, Nov 13, 2024 at 5:15 PM Hemant Agrawal <hemant.agra...@nxp.com> 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: sta...@dpdk.org
>
> Signed-off-by: Hemant Agrawal <hemant.agra...@nxp.com>
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 = (fq); \
- if (fq_isset(__fq478, QMAN_FQ_FLAG_LOCKED)) \
- spin_lock(&__fq478->fqlock); \
- } while (0)
-#define FQUNLOCK(fq) \
- do { \
- struct qman_fq *__fq478 = (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 += include_directories('include', 'base/qbman')
-
-annotate_locks = false
--
David Marchand
