From nobody Mon Jul 17 17:52:07 2023 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 4R4V5y3QD2z4ndNd for ; Mon, 17 Jul 2023 17:52:22 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) (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 "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4R4V5x6Fvqz4lPG for ; Mon, 17 Jul 2023 17:52:21 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ed1-x535.google.com with SMTP id 4fb4d7f45d1cf-51e28cac164so12462724a12.1 for ; Mon, 17 Jul 2023 10:52:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20221208.gappssmtp.com; s=20221208; t=1689616339; x=1692208339; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=GjjyCks/+P0ZDesVVFSCCa3rc4u1l3rfKIb3ELGp3/M=; b=CSXSC3tv+3rxwKVmgHDK37mBgEBa/eBiWi3jFv9p29LEv5CuX2I1xGz3KOXNq1w9t6 IHl7/TzX7tDUftFSo5mSKS06b04a6Js0XGiejRqH9XeBwIroaoQDv1IpAi+M8yBJcau3 YPzhMjl0UzjrrJsfnrWS/OGvFWGmRo2Ilk3rtKNS15Ck+lFMYMqfxJaDGzYO23MtZNIm M7XGuNz5jxmmCAFy5iMoQKCCYKEEV7iTMdS0KYbx6Be1EvX7PrTtQRYB3sCdtx7LYGOL eTy4hCzmUo+9AuR9H28cgXWp3Jt/z/2nxCmhC3w4VZFsQ4i+UjEp1fKnzDKLUnSSTdEH Nnwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689616339; x=1692208339; h=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=GjjyCks/+P0ZDesVVFSCCa3rc4u1l3rfKIb3ELGp3/M=; b=llLFs6w0fK6TfqFFYxwFGSPQztlclIUZK8s/c9kAxrGDQPqsn1BSYopIwQxNQH11zY dEg/9RMvF7nA6EJa0aK2BGmkhQ5Ojz02FgqiyQCZZJcUR6raEzFsg3wWYW9RB2FdJ09v a+VDJTRoQk7c/DDpqu20DhVojaaNRZv0p+X1FNXQbbjdCyYKm9AbdpigxwBXNIH/kxf5 YTalLz8ppDzjAY1HJlmg8LnnCqlDhO6QCJLmM46OrTI/1wCx9mCekAH0ToqssR8FylyE AED4WbMJpfZs9/RuXmHexv52PsfgdNBuMg7aU8ixk9H+3QCIjb/kTy50PT2NjUkD+IUg VprA== X-Gm-Message-State: ABy/qLZM3iZUbeeKFCz9ll/p6A192O8SibBEVf8/sBi+8r3a6V26Q1Lb nV74B2E+ymWEmaEdIAsENcVL8/p/zNcEYd5cPBBKlQ== X-Google-Smtp-Source: APBJJlEvnLU7u6Pm8d7JAZ0yj8dIXQ8ps8weJaLhSprkarqTBYppST3unPNmmzH4gCcaDjSzIRu662zWrpuH8uzjaiU= X-Received: by 2002:a05:6402:42cc:b0:51f:e0f0:f2cd with SMTP id i12-20020a05640242cc00b0051fe0f0f2cdmr12173297edc.3.1689616338839; Mon, 17 Jul 2023 10:52:18 -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: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202307141841.36EIf3f0019403@gitrepo.freebsd.org> <65d7d8d8-9f98-abd2-1ce3-ae3a2d3bf111@FreeBSD.org> <17cd00bb-c6f2-5afa-a1ac-b7e14c3c758e@FreeBSD.org> In-Reply-To: <17cd00bb-c6f2-5afa-a1ac-b7e14c3c758e@FreeBSD.org> From: Warner Losh Date: Mon, 17 Jul 2023 11:52:07 -0600 Message-ID: Subject: Re: git: 60381fd1ee86 - main - memdesc: Retire MEMDESC_CCB. To: John Baldwin Cc: Konstantin Belousov , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="000000000000aefcf40600b2750f" X-Rspamd-Queue-Id: 4R4V5x6Fvqz4lPG X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N --000000000000aefcf40600b2750f Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Jul 17, 2023 at 11:45=E2=80=AFAM John Baldwin wro= te: > On 7/17/23 10:29 AM, Warner Losh wrote: > > On Mon, Jul 17, 2023 at 11:15=E2=80=AFAM John Baldwin = wrote: > > > >> On 7/17/23 8:48 AM, Konstantin Belousov wrote: > >>> On Fri, Jul 14, 2023 at 06:41:03PM +0000, John Baldwin wrote: > >>>> The branch main has been updated by jhb: > >>>> > >>>> URL: > >> > https://cgit.FreeBSD.org/src/commit/?id=3D60381fd1ee8668ea1e4676a6128883d= 987cab858 > >>>> > >>>> commit 60381fd1ee8668ea1e4676a6128883d987cab858 > >>>> Author: John Baldwin > >>>> AuthorDate: 2023-07-14 18:30:31 +0000 > >>>> Commit: John Baldwin > >>>> CommitDate: 2023-07-14 18:32:16 +0000 > >>>> > >>>> memdesc: Retire MEMDESC_CCB. > >>>> > >>>> Instead, change memdesc_ccb to examine the CCB and return a > >> memdesc of > >>>> a more generic type describing the data buffer. > >>> > >>>> diff --git a/sys/kern/subr_bus_dma.c b/sys/kern/subr_bus_dma.c > >>>> index 65a08aeba17c..bfaad30b37d3 100644 > >>>> --- a/sys/kern/subr_bus_dma.c > >>>> +++ b/sys/kern/subr_bus_dma.c > >>>> @@ -304,94 +304,6 @@ bus_dmamap_load_ma_triv(bus_dma_tag_t dmat, > >> bus_dmamap_t map, > >>>> @@ -566,49 +478,18 @@ bus_dmamap_load_ccb(bus_dma_tag_t dmat, > >> bus_dmamap_t map, union ccb *ccb, > >>>> + mem =3D memdesc_ccb(ccb); > >>>> + return (bus_dmamap_load_mem(dmat, map, &mem, callback, > >> callback_arg, > >>>> + flags)); > >>>> } > >>> This makes kernel not linkable if CAM is not included into it. > >> > >> Hmmm, ok. I can either move the memdesc_ccb routine into sys/kern > >> somewhere > >> (like the kern_memdesc.c file in my other pending review), or we can > #ifdef > >> this function. It probably doesn't make sense to have a > >> bus_dmamap_load_ccb > >> if you don't have CAM, so I think I prefer the second option. > >> > > > > MINIMAL doesn't have CAM configured, but it is loadable as a module. > > > > I'd think we'd want a dummy one fo these with weak symbol binding and > have > > the actual > > one live in cam somewhere that overrides this symbol. > > > > I just hit this in building MINIMAL for other reasons.... > > Yeah, I was testing MINIMAL locally (which is still broken due to recent > TCP commits) to try out possible fixes. One could possibly move > bus_dmamap_load_ccb into cam.ko entirely? Especially this current versio= n > doesn't really have any bus_dma-specific knowledge anymore but is just a > thin wrapper around bus_dmamap_load_mem: > > int > bus_dmamap_load_ccb(bus_dma_tag_t dmat, bus_dmamap_t map, union ccb *ccb, > bus_dmamap_callback_t *callback, void *callback_arg, > int flags) > { > struct ccb_hdr *ccb_h; > struct memdesc mem; > > ccb_h =3D &ccb->ccb_h; > if ((ccb_h->flags & CAM_DIR_MASK) =3D=3D CAM_DIR_NONE) { > callback(callback_arg, NULL, 0, 0); > return (0); > } > > mem =3D memdesc_ccb(ccb); > return (bus_dmamap_load_mem(dmat, map, &mem, callback, > callback_arg, > flags)); > } > And who calls bus_dmamap_load_ccb? If this were entirely in cam.ko, then callers of it would need to resolve it. sys/dev/nvme/nvme_qpair.c is not otherwise dependent on cam, and would be unresolved for a minimal + nvme + nvd kernel. Warner --000000000000aefcf40600b2750f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Jul 17, 2023 at 11:45=E2=80= =AFAM John Baldwin <jhb@freebsd.org> wrote:
On= 7/17/23 10:29 AM, Warner Losh wrote:
> On Mon, Jul 17, 2023 at 11:15=E2=80=AFAM John Baldwin <jhb@freebsd.org> wrote:
>
>> On 7/17/23 8:48 AM, Konstantin Belousov wrote:
>>> On Fri, Jul 14, 2023 at 06:41:03PM +0000, John Baldwin wrote:<= br> >>>> The branch main has been updated by jhb:
>>>>
>>>> URL:
>> https://c= git.FreeBSD.org/src/commit/?id=3D60381fd1ee8668ea1e4676a6128883d987cab858
>>>>
>>>> commit 60381fd1ee8668ea1e4676a6128883d987cab858
>>>> Author:=C2=A0 =C2=A0 =C2=A0John Baldwin <jhb@FreeBSD.or= g>
>>>> AuthorDate: 2023-07-14 18:30:31 +0000
>>>> Commit:=C2=A0 =C2=A0 =C2=A0John Baldwin <jhb@FreeBSD.or= g>
>>>> CommitDate: 2023-07-14 18:32:16 +0000
>>>>
>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0memdesc: Retire MEMDESC_CCB.
>>>>
>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0Instead, change memdesc_ccb to e= xamine the CCB and return a
>> memdesc of
>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0a more generic type describing t= he data buffer.
>>>
>>>> diff --git a/sys/kern/subr_bus_dma.c b/sys/kern/subr_bus_d= ma.c
>>>> index 65a08aeba17c..bfaad30b37d3 100644
>>>> --- a/sys/kern/subr_bus_dma.c
>>>> +++ b/sys/kern/subr_bus_dma.c
>>>> @@ -304,94 +304,6 @@ bus_dmamap_load_ma_triv(bus_dma_tag_t= dmat,
>> bus_dmamap_t map,
>>>> @@ -566,49 +478,18 @@ bus_dmamap_load_ccb(bus_dma_tag_t dm= at,
>> bus_dmamap_t map, union ccb *ccb,
>>>> +=C2=A0 =C2=A0 mem =3D memdesc_ccb(ccb);
>>>> +=C2=A0 =C2=A0 return (bus_dmamap_load_mem(dmat, map, &= ;mem, callback,
>> callback_arg,
>>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 flags));
>>>>=C2=A0 =C2=A0 }
>>> This makes kernel not linkable if CAM is not included into it.=
>>
>> Hmmm, ok.=C2=A0 I can either move the memdesc_ccb routine into sys= /kern
>> somewhere
>> (like the kern_memdesc.c file in my other pending review), or we c= an #ifdef
>> this function.=C2=A0 It probably doesn't make sense to have a<= br> >> bus_dmamap_load_ccb
>> if you don't have CAM, so I think I prefer the second option.<= br> >>
>
> MINIMAL doesn't have CAM configured, but it is loadable as a modul= e.
>
> I'd think we'd want a dummy one fo these with weak symbol bind= ing and have
> the actual
> one live in cam somewhere that overrides this=C2=A0 symbol.
>
> I just hit this in building MINIMAL for other reasons....

Yeah, I was testing MINIMAL locally (which is still broken due to recent TCP commits) to try out possible fixes.=C2=A0 One could possibly move
bus_dmamap_load_ccb into cam.ko entirely?=C2=A0 Especially this current ver= sion
doesn't really have any bus_dma-specific knowledge anymore but is just = a
thin wrapper around bus_dmamap_load_mem:

int
bus_dmamap_load_ccb(bus_dma_tag_t dmat, bus_dmamap_t map, union ccb *ccb, =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bus_d= mamap_callback_t *callback, void *callback_arg,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 int f= lags)
{
=C2=A0 =C2=A0 =C2=A0 =C2=A0 struct ccb_hdr *ccb_h;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 struct memdesc mem;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 ccb_h =3D &ccb->ccb_h;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((ccb_h->flags & CAM_DIR_MASK) =3D=3D= CAM_DIR_NONE) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 callback(callback_a= rg, NULL, 0, 0);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (0);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }

=C2=A0 =C2=A0 =C2=A0 =C2=A0 mem =3D memdesc_ccb(ccb);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return (bus_dmamap_load_mem(dmat, map, &mem= , callback, callback_arg,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 flags));
}


--000000000000aefcf40600b2750f--