From nobody Wed Jun 05 17:52:02 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VvZmH6l0tz5LPsK for ; Wed, 05 Jun 2024 17:52:15 +0000 (UTC) (envelope-from kevin.bowling@kev009.com) Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VvZmH53D3z4Tsk for ; Wed, 5 Jun 2024 17:52:15 +0000 (UTC) (envelope-from kevin.bowling@kev009.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-pj1-x102c.google.com with SMTP id 98e67ed59e1d1-2c1ab9e17f6so85338a91.1 for ; Wed, 05 Jun 2024 10:52:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kev009.com; s=google; t=1717609934; x=1718214734; darn=freebsd.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=3VGcqv+30ZwjpbLu6KT3vHBcWHAogQT/fJD2Z6JO8ls=; b=CaKHkCKMvZjgLW/zfCbzDgpDou8AO1UuMRXUqf4mRl+rwEho0B5/jK9X4f7F69Dada 0c74SXLYPzC+kT82EjlzyPQoLcKcbJymg9emTSprOxxE6vfsaAktRvM/mQkEIM0eTaxP SyQjxmghB+i3sdIUbXawp/DCFe7sJppVrcG/Y= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717609934; x=1718214734; 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=3VGcqv+30ZwjpbLu6KT3vHBcWHAogQT/fJD2Z6JO8ls=; b=eF6x9jq4FsNig+c4q+qGGEOHRFqqALAvqwiZSRCFsRg32k4HYlVguq8tvkwEof6oBK Q+SnxjcSTC68O8Ed6G7o9r6yxDbB6mbg/Ynet0niOWulUcLz9832TXU0upIiY0qKZC3W mHozgU9FwddPapRwj4+EVmXisIDPFYFKSUbR+2xPm8EUZf71kpR32cdLZ9CqG30VIEOr wlPITsnipq58yeJOKGu69Bt7oC3P4+ZNriBfmAvAog4auqNpTShyo3JSs0XlxnnS/Gtv 2ukcHqONI5cSZGhhgqVsYcVs2sCJ5wozHnfaEkSBOBlTqRKtDjnlC8F7zCUBQGdPuw/2 vB+w== X-Forwarded-Encrypted: i=1; AJvYcCUr9LrkYHvbZ7+7K8jOaomGCKRzfuQCT7zaQswwsq87g0OZSehpCdYv844GbezqP51JWg9fs7omBN0yIkU814wMfkzDv1dfepYvPpfukWeL X-Gm-Message-State: AOJu0YxueA1x2UUXbOBiDlBjE9Y3ehGMUZHrgTr8rGW/CJQUd4cl1U+M YQY2ypObYvFYA3Uore839Q8ZdwYXu4OtwzKKRkimKJxccIPGZyNPKHQt8ph6Mwwt0nyhCtwyhnk 9vJh14jaibKUWgfoCV8CTjlnxNr5EFtgkoJrO X-Google-Smtp-Source: AGHT+IH2+eGCk6UBVrcpn2zRvhvz9yOm6DPxB3J8E7pAxafwbJE5qA6aej3LBULvcJzfvHeZzzmsxRjD036WPvRagqg= X-Received: by 2002:a17:90a:e392:b0:2c2:97c2:1426 with SMTP id 98e67ed59e1d1-2c297c2156bmr466794a91.10.1717609934210; Wed, 05 Jun 2024 10:52:14 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 References: <202405280643.44S6hBkh054482@gitrepo.freebsd.org> <1ade77c1-f15a-4e20-a8ea-29ac3a86e85e@FreeBSD.org> In-Reply-To: <1ade77c1-f15a-4e20-a8ea-29ac3a86e85e@FreeBSD.org> From: Kevin Bowling Date: Wed, 5 Jun 2024 10:52:02 -0700 Message-ID: Subject: Re: git: 1c45a62a2f66 - main - qlnxe: Fix multiple locking issues To: John Baldwin , keith.reynolds@hpe.com Cc: Kevin Bowling , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] X-Rspamd-Queue-Id: 4VvZmH53D3z4Tsk On Tue, Jun 4, 2024 at 4:02=E2=80=AFPM John Baldwin wrote= : > > On 5/28/24 2:43 AM, Kevin Bowling wrote: > > The branch main has been updated by kbowling: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D1c45a62a2f667b45ec10a92a= d58ff5a34e68b569 > > > > commit 1c45a62a2f667b45ec10a92ad58ff5a34e68b569 > > Author: Keith Reynolds > > AuthorDate: 2024-05-28 06:41:05 +0000 > > Commit: Kevin Bowling > > CommitDate: 2024-05-28 06:41:05 +0000 > > > > qlnxe: Fix multiple locking issues > > > > Multiple issues are reported with WITNESS and code inspection of t= he > > locking and lock initialization. > > > > PR: 278084 > > MFC after: 1 week > > --- > > sys/dev/qlnx/qlnxe/bcm_osal.h | 8 +++---- > > sys/dev/qlnx/qlnxe/ecore.h | 1 + > > sys/dev/qlnx/qlnxe/ecore_mcp.c | 48 +++++++++++++++++++++------------= --------- > > sys/dev/qlnx/qlnxe/ecore_mcp.h | 6 +++--- > > sys/dev/qlnx/qlnxe/qlnx_def.h | 2 +- > > sys/dev/qlnx/qlnxe/qlnx_os.c | 9 ++++---- > > sys/dev/qlnx/qlnxe/qlnx_os.h | 4 ++-- > > 7 files changed, 40 insertions(+), 38 deletions(-) > > > > diff --git a/sys/dev/qlnx/qlnxe/bcm_osal.h b/sys/dev/qlnx/qlnxe/bcm_osa= l.h > > index 5d940d3272d6..c820532c9e0a 100644 > > --- a/sys/dev/qlnx/qlnxe/bcm_osal.h > > +++ b/sys/dev/qlnx/qlnxe/bcm_osal.h > > @@ -72,7 +72,7 @@ extern void qlnx_dma_free_coherent(void *ecore_dev, v= oid *v_addr, > > bus_addr_t phys, uint32_t size); > > > > extern void qlnx_link_update(void *p_hwfn); > > -extern void qlnx_barrier(void *p_hwfn); > > +extern void qlnx_barrier(void *p_dev); > > > > extern void *qlnx_zalloc(uint32_t size); > > > > @@ -213,14 +213,14 @@ typedef struct osal_list_t > > #define OSAL_SPIN_LOCK_ALLOC(p_hwfn, mutex) > > #define OSAL_SPIN_LOCK_DEALLOC(mutex) mtx_destroy(mutex) > > #define OSAL_SPIN_LOCK_INIT(lock) {\ > > - mtx_init(lock, __func__, MTX_NETWORK_LOCK, MTX_SPIN); \ > > + mtx_init(lock, __func__, "OSAL spin lock", MTX_SPIN); \ > > } > > > > Do you really need MTX_SPIN here? Device drivers rarely need spin locks. > The equivalent to a Linux spin lock in drivers is generally a MTX_DEF > mutex. Can you include the author for follow ups? This driver was a bit of a disaster, the patch in question doesn't rectify that overall but does fix the most serious PRs which is worthwhile. The spin in question seems to match what the vendor initially did in their OS API, Keith may be able to test your suggestion. > -- > John Baldwin >