From nobody Mon Jul 17 17:58:55 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 4R4VFn3x9Nz4nfpd for ; Mon, 17 Jul 2023 17:59:09 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-lf1-x12d.google.com (mail-lf1-x12d.google.com [IPv6:2a00:1450:4864:20::12d]) (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 4R4VFm5MNnz3DxV for ; Mon, 17 Jul 2023 17:59:08 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-lf1-x12d.google.com with SMTP id 2adb3069b0e04-4fb7769f15aso7701841e87.0 for ; Mon, 17 Jul 2023 10:59:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20221208.gappssmtp.com; s=20221208; t=1689616747; x=1692208747; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=6LUpG4c0ikXDgzFkpX7egKXQVV83YcFRAfzVaMLfaqU=; b=sy5fNQOW9de8NaKoJj5NyyQX1duL2UX1x7GkTaJafMPuPQlwLyZuhIb8CN1VP7DHFC NXcA/9/8Kj7+sl7Ke2ohWXBlkOkbpQ/lodtNdAJiH2iK+rEgskUZEc9D8PL1a0oIIGmu TJxJqz+DXnCjlJDEPe/lg3BrTg8yN+xRafjZs8iuGECjmAX/f3QJR3Ibp4EBnOFrqqTW T2bGOUybzLNPyONSLPLh8AZe7+CuE9ys4dlm07uWrwoUYO7vQxm1AZ3mX4bLkHyPFPX3 y2i3Kl9DHegbw9uM45zyjZf/KvAh+WTPrub+ET7ObxNu6rrjUrUHbJjQMJ4qVcgp9iZV W14g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689616747; x=1692208747; 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=6LUpG4c0ikXDgzFkpX7egKXQVV83YcFRAfzVaMLfaqU=; b=OdMKiptXTj0x7zJfNUvypw0thG9QMPDbpLvs1zAYayyO05Hvr0AnOh1KNtJ04ykpBG J+QJPuJY0xQxERa8S96Td1Pmh9FYVvQY2A9Gohm073E1iDrZgHpne8PLrr9UIjaPJ0ga 8wmBSFblaD5/AFpqIm9frrhc2CuhyCcBD7JC9IIm+CJaTx1Vw0xe09WS7/s4Yrctex7k NV/CUwmyzK3GwI+34Pcs+wF+PiJ+zJwSRe2/gHJG/G6k1cRF9/mSfijQYoDeLY8MKBH1 6rT7/xbX64mR/FnVpGqZH3NPuadp1sQcR0gDTxtIWhRAn9dBC+C0QgYVKlLMbAabn3jp /tjA== X-Gm-Message-State: ABy/qLYDRlQAzlmIYI3V5vQFvC9geJL4Skh/Z+6IVfTPh40Lyb2tSpPI pT66S2/fUOt92aRFP1RIJlMw+n1keSls8f9QC4RjFDJMTTHA8tIO57U= X-Google-Smtp-Source: APBJJlHpnRwGHk60z00GXu6MGeGQnaUcXhVF3arG9Xe+IWYul4Rrxs0Cvj981oEUfOVVjXuVUkodYgJg3Wxm3ZOXK8E= X-Received: by 2002:a05:6512:705:b0:4f8:4245:ed57 with SMTP id b5-20020a056512070500b004f84245ed57mr8245485lfs.35.1689616746540; Mon, 17 Jul 2023 10:59:06 -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> In-Reply-To: From: Warner Losh Date: Mon, 17 Jul 2023 11:58:55 -0600 Message-ID: Subject: Re: git: 60381fd1ee86 - main - memdesc: Retire MEMDESC_CCB. To: Konstantin Belousov Cc: John Baldwin , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="000000000000fc0c310600b28d2b" X-Rspamd-Queue-Id: 4R4VFm5MNnz3DxV 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 --000000000000fc0c310600b28d2b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Jul 17, 2023 at 11:54=E2=80=AFAM Konstantin Belousov wrote: > On Mon, Jul 17, 2023 at 11:29:20AM -0600, 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. > The symbol resolution does not work this way in kernel. And it cannot > made working this way even in theory, because cam.ko is loadable at > runtime. > Yea... It could, if we have perfect knowledge of all the places that it is called. But I don't think we do, now that I think about it... so yes, it's good to want things, but in this case my desire cannot exist, I agree. > I believe the only feasible solution is to move memdesc_ccb() into kernel > unconditionally. > Or if it was in cam.h and made a static inline. It's short enough that won't bloat the kernel in the half a dozen places its called, and it would give similar performance to what we have today with the half a dozen nearly identical copies of this routine. And since it's all done with structure dancing, there's no other bits of CAM that would be brought into the kernel. Warner --000000000000fc0c310600b28d2b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Jul 17, 2023 at 11:54=E2=80= =AFAM Konstantin Belousov <kostik= bel@gmail.com> wrote:
On Mon, Jul 17, 2023 at 11:29:20AM -0600, 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= :
> > >> The branch main has been updated by jhb:
> > >>
> > >> URL:
> > https://= cgit.FreeBSD.org/src/commit/?id=3D60381fd1ee8668ea1e4676a6128883d987cab858<= /a>
> > >>
> > >> commit 60381fd1ee8668ea1e4676a6128883d987cab858
> > >> Author:=C2=A0 =C2=A0 =C2=A0John Baldwin <jhb@FreeBSD.= org>
> > >> AuthorDate: 2023-07-14 18:30:31 +0000
> > >> Commit:=C2=A0 =C2=A0 =C2=A0John Baldwin <jhb@FreeBSD.= org>
> > >> CommitDate: 2023-07-14 18:32:16 +0000
> > >>
> > >>=C2=A0 =C2=A0 =C2=A0 memdesc: Retire MEMDESC_CCB.
> > >>
> > >>=C2=A0 =C2=A0 =C2=A0 Instead, change memdesc_ccb to exami= ne the CCB and return a
> > memdesc of
> > >>=C2=A0 =C2=A0 =C2=A0 a more generic type describing the d= ata 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,
> > >> +=C2=A0 =C2=A0 mem =3D memdesc_ccb(ccb);
> > >> +=C2=A0 =C2=A0 return (bus_dmamap_load_mem(dmat, map, &a= mp;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 i= t.
> >
> > Hmmm, ok.=C2=A0 I can either move the memdesc_ccb routine into sy= s/kern
> > somewhere
> > (like the kern_memdesc.c file in my other pending review), or we = can #ifdef
> > this function.=C2=A0 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 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.
The symbol resolution does not work this way in kernel.=C2=A0 And it cannot=
made working this way even in theory, because cam.ko is loadable at
runtime.

<= div>=C2=A0
I believe the only feasible solution is to move memdesc_ccb() into kernel unconditionally.

<= div>the kernel in the half a dozen places its called, and it would give sim= ilar performance
--000000000000fc0c310600b28d2b--