From nobody Fri Jun 6 21:37:39 2025 X-Original-To: dev-commits-src-main@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 4bDZRp1y19z5y2Yk for ; Fri, 06 Jun 2025 21:37:58 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-yw1-x1134.google.com (mail-yw1-x1134.google.com [IPv6:2607:f8b0:4864:20::1134]) (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 4bDZRn40VDz3wRQ for ; Fri, 06 Jun 2025 21:37:57 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-yw1-x1134.google.com with SMTP id 00721157ae682-70e5599b795so26817027b3.3 for ; Fri, 06 Jun 2025 14:37:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1749245872; x=1749850672; 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=Zga1Sz3BLcwWlEYiC2uBZiyDifpefMebRMwohKPAgUU=; b=HwVyCq1Msx0mPViZAXICCJ2ThvzEV8NrwwSiN+TyyVC96W1x2LdxHeT/WvLrYDhjuQ QrQrtAWPdEFzyVjddQNzWqh6CwO+NnraWnTQTZK75VfBswdu253iCYxFEqbxJaaPDEH9 w9D/To/3s79NTOlvHDubZYBGtvWSEj5JTsYgECJsSiDgwNZKX8mk8TkFy5Xu5osbQrWb 7345Jb8uztYEQfYeRo3lQv4WRaiRabbg+Fp0SUVREwM8WfOXk1rKD4rRGaSt9Yxjo3t7 LfF2qZbN1L1vAW1poAp3Un7eJlwdigH236MX5FOW0/hVUDnjXWx6dTKk4DeHtuThn5uC N1DA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1749245872; x=1749850672; 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=Zga1Sz3BLcwWlEYiC2uBZiyDifpefMebRMwohKPAgUU=; b=Q32OpuNCNpG6SOB+xNTVZHoTDLDhAnCUdQlfS6Z69qThfQ9ZlQ9ga4ch03+n9+X4Q2 UY+ZJ6kP9KEnKeCkNrD+8k9kJZb5a1kWmUcOYLmSKb2/4gGtEPs/niplID2taaHT2oEF vdBNMeNL+cM5tK7XCtZGNIVmxjab4FvktyN1MFSzxEAWzdovnQ660Dd6U0ULTw+b8mJm HzdJZ/cMGnOiYOKuJO5TAAy9r0Z4hT4DI6PdT3OTN0QWCpwBnxOpxmOaqYUmKTt+ffKN wKuVXpN8IZaW3Qo+NHz8B/I7btoGyUkZ+qW0xFZg8PCo+CXfz4w1RhsKYTUKFSg+1FJ3 ZZLQ== X-Forwarded-Encrypted: i=1; AJvYcCUKxT5Y1R79C3Z06m834D3qWVIr1NbLwg4JAE5vltTSB+0sbCKhho5ETEfyoayXIOulflVT1QPvWLA88DLnRR53B6LLlw==@freebsd.org X-Gm-Message-State: AOJu0Yx8ONiVof8hV58JZV02UtksNSQbP8LqRLPvK7Hr/YfK/K7Gt3kX QyDl5W43Ne3iDAXOPXKqQ6U46mhINotB3GSZRmybiG8Yd0NihOo63AQfxb9Xmx9IksdPPQruIsr 0DivZ6GjdwpEdv0KjSGV9q1ofvJU5/UTmx+GFcBXbbQ== X-Gm-Gg: ASbGnct6fFAyd6g5RxtJKGllX2cFJ8jUNKikf9AxPtPhbDmRMGCPF/KNJQv/uxZOHEB fwNsB4jd7jTAlyCYV2w4ZVLH5WWwFqVw+MLYPFz+FBzxDdJIsu0q8GmZ1cbDn1QHCGWNHKq94XE 3ZJqWmJAe2Y3M+btgnl1m6qZyW70wlGDZhadmrGHoHp4b9L3J61+nSOQ== X-Google-Smtp-Source: AGHT+IGWaoscmwkw/GfIroo3Mixu6IN7UnGQX23OkR/K+OAz9hUJRBMvzuVs38aq8Knmeyz8RJNPPszVKgCdzAw8qnU= X-Received: by 2002:a05:690c:6d0b:b0:70d:ed5d:b4d2 with SMTP id 00721157ae682-710f76949femr79778647b3.16.1749245871740; Fri, 06 Jun 2025 14:37:51 -0700 (PDT) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 References: <202506060132.5561Wakc094185@gitrepo.freebsd.org> <346c0310-b9b6-4eb7-ac94-0575a83c3416@FreeBSD.org> In-Reply-To: <346c0310-b9b6-4eb7-ac94-0575a83c3416@FreeBSD.org> From: Warner Losh Date: Fri, 6 Jun 2025 15:37:39 -0600 X-Gm-Features: AX0GCFu0noPrayLc4cMjmVzLnFdJfMOq9gMTwXOzw9oPpe_J3mmi4qkgPD4sNsE Message-ID: Subject: Re: git: 7b3ee39e73af - main - libcam: Include nvme opcode and status code routines from nvme_util.c To: John Baldwin Cc: 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-Rspamd-Queue-Id: 4bDZRn40VDz3wRQ 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] On Fri, Jun 6, 2025 at 8:34=E2=80=AFAM John Baldwin wrote= : > > On 6/5/25 22:21, Warner Losh wrote: > > On Thu, Jun 5, 2025 at 7:41=E2=80=AFPM John Baldwin w= rote: > >> > >> On 6/5/25 21:32, John Baldwin wrote: > >>> The branch main has been updated by jhb: > >>> > >>> URL: https://cgit.FreeBSD.org/src/commit/?id=3D7b3ee39e73af36f49f471f= 7900baeb98ac3504d0 > >>> > >>> commit 7b3ee39e73af36f49f471f7900baeb98ac3504d0 > >>> Author: John Baldwin > >>> AuthorDate: 2025-06-06 01:28:38 +0000 > >>> Commit: John Baldwin > >>> CommitDate: 2025-06-06 01:28:38 +0000 > >>> > >>> libcam: Include nvme opcode and status code routines from nvme_= util.c > >>> > >>> libcam in userspace also includes nvme_all.c which now depends = on > >>> nvme_util.c, so add nvme_util.c to libcam's sources. This requ= ires > >>> exporting the opcode and status code routines in nvme_util.c to > >>> userspace as well as the kernel. In turn, this means nvmecontr= ol now > >>> depends on libsbuf (which is already present in /lib). > >>> > >>> Reported by: viswhin, Jenkins > >>> Fixes: 60159a98a837 ("nvme: Move opcode and status cod= e tables from base CAM to nvme_util.c") > >>> Sponsored by: Chelsio Communications > >> > >> This fixes the build for now (and sorry for breaking it). However, th= is > >> raises a few questions for me at least. Why does libcam include nvme_= all.c > >> at all? We don't document any of the nvme_* functions in cam(3), nor = any > >> of the functions from scsi_all.c, smp_all.c, etc. This seems really o= dd, > >> and it also means that we can add (and remove!) symbols from libcam wi= thout > >> realizing it by changing sys/cam//_all.h which is not ve= ry > >> obvious. > >> > >> It seems to me that we should be more intentional about which symbols = we > >> export from libcam. Switching to symbol versioning (which implicitly > >> enforces hidden visibility on all symbols not explicitly exported) wou= ld > >> keep us from leaking symbols into the ABI of libcam that we don't inte= nd > >> to export. > >> > >> I also wonder if we can remove some of the *_all.c files from libcam > >> entirely? None of the nvme_* ones are used outside of the kernel in t= he > >> base system for example. > > > > No. We can't remove all that. They are used by camcontrol, ddcam and > > others. We use the functions in *_all.h/c functions to populate the > > CCBs to send down into the kernel. I think that we need a different > > take on it. It is, after all, basically designed in FreeBSD 3 > > timeframe when such considerations were the furthest thing from the > > minds of the developers. Times have changed, though, and libcam hasn't > > with it. > > We don't use a single routine from nvme_all.c outside of the kernel. > libcam also doesn't include mmc_all.c. Hmmm, I added it into libcam because we did use one. But maybe we don't need it. It's been a while, and I added it in my experimental tree and push that upstream before I pushed the things that used it, so maybe something got dropped. > > We don't document it very well... Sometimes I've thought we should > > make libcam a private library. But I don't know who all uses it in raw > > mode. I think about a dozen ports use this for things like smart > > reporting... > > I think a decent start would be to maybe be a bit more choosy in what > things have to be exported. Only a few routines in ata_all.c and > smp_all.c appear to be used in camcontrol for example. And it > certainly seems like we could remove nvme_all.c from libcam entirely. I'll give that a short. I was sure that something was using it, but that may have only been in early version(s) of code... I'll look into it. > Using a version map with symbol versioning might be the best way to > enforce that (though more liberal use of #ifdef _KERNEL in *_all.c > would be another route). We definitely will need to bump the > SHLIB_MAJOR for libcam before branching 15 though. Yea. I've been hesitant to clean this up too much. I know $WORK doesn't need this library for anything interesting. But I also think we should check with Netapp and Spectralogic. Warner