Date: Tue, 18 Jul 2023 19:44:24 -0600 From: Warner Losh <imp@bsdimp.com> To: John Baldwin <jhb@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: c5312bd79e66 - main - cam: Move bus_dmamap_load_ccb into cam.c. Message-ID: <CANCZdfotvSACUJshOLDR-qd97z1BTwgeYqtYrFYW6-_wSzL1CQ@mail.gmail.com> In-Reply-To: <202307190120.36J1K1mQ011397@gitrepo.freebsd.org> References: <202307190120.36J1K1mQ011397@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--00000000000075cc400600cd2ca2 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable As predicted in the review, this is broken: -- command output -- linking kernel.full ld: error: undefined symbol: bus_dmamap_load_ccb >>> referenced by nvme_qpair.c:1209 (/usr/home/imp/git/freebsd/src/sys/dev/nvme/nvme_qpair.c:1209) >>> nvme_qpair.o:(_nvme_qpair_submit_request) _ from using sys/amd64/conf/EX include MINIMAL device nvme device nvd This has to be in the header file. The MODULE_DEPENDS stuff doesn't pull anything in for the static kernel case. Please, lets' do this in the header with a static inline like I suggested to get around this issue. Warner On Tue, Jul 18, 2023 at 7:20=E2=80=AFPM John Baldwin <jhb@freebsd.org> wrot= e: > The branch main has been updated by jhb: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3Dc5312bd79e66ce8ef50655ce7f3eca0= 6d6b6e24f > > commit c5312bd79e66ce8ef50655ce7f3eca06d6b6e24f > Author: John Baldwin <jhb@FreeBSD.org> > AuthorDate: 2023-07-19 01:19:27 +0000 > Commit: John Baldwin <jhb@FreeBSD.org> > CommitDate: 2023-07-19 01:19:27 +0000 > > cam: Move bus_dmamap_load_ccb into cam.c. > > This routine is specific to CAM and no longer assumes any internal > bus_dma knowledge as it is simple wrapper around bus_dmamap_load_mem. > > Fixes: 60381fd1ee86 memdesc: Retire MEMDESC_CCB. > Reviewed by: kib > Differential Revision: https://reviews.freebsd.org/D41058 > --- > sys/cam/cam.c | 20 ++++++++++++++++++++ > sys/kern/subr_bus_dma.c | 19 ------------------- > 2 files changed, 20 insertions(+), 19 deletions(-) > > diff --git a/sys/cam/cam.c b/sys/cam/cam.c > index ce7dc81b3495..7d9d8602d009 100644 > --- a/sys/cam/cam.c > +++ b/sys/cam/cam.c > @@ -52,6 +52,7 @@ __FBSDID("$FreeBSD$"); > > #ifdef _KERNEL > #include <sys/libkern.h> > +#include <machine/bus.h> > #include <cam/cam_queue.h> > #include <cam/cam_xpt.h> > > @@ -642,4 +643,23 @@ memdesc_ccb(union ccb *ccb) > panic("%s: flags 0x%X unimplemented", __func__, > ccb_h->flags); > } > } > + > +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)); > +} > #endif > diff --git a/sys/kern/subr_bus_dma.c b/sys/kern/subr_bus_dma.c > index da7a2ee4cdc9..683b41d0047c 100644 > --- a/sys/kern/subr_bus_dma.c > +++ b/sys/kern/subr_bus_dma.c > @@ -449,25 +449,6 @@ bus_dmamap_load_uio(bus_dma_tag_t dmat, bus_dmamap_t > map, struct uio *uio, > return (error); > } > > -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)); > -} > - > int > bus_dmamap_load_bio(bus_dma_tag_t dmat, bus_dmamap_t map, struct bio *bi= o, > bus_dmamap_callback_t *callback, void *callback_arg, > --00000000000075cc400600cd2ca2 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><div>As predicted in the review, this is = broken:</div><div><br></div><div>-- command output --<br>linking kernel.ful= l<br>ld: error: undefined symbol: bus_dmamap_load_ccb<br>>>> refer= enced by nvme_qpair.c:1209 (/usr/home/imp/git/freebsd/src/sys/dev/nvme/nvme= _qpair.c:1209)<br>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 nvme_qpair.o:(_nvme_qpair_submit_request)<br>_</div><div>from using = sys/amd64/conf/EX <br>include MINIMAL<br>device nvme<br>device nvd<br></div= ></div><br><div class=3D"gmail_quote"><div class=3D"gmail_attr">This has to= be in the header file. The MODULE_DEPENDS stuff doesn't pull anything = in for the</div><div class=3D"gmail_attr">static kernel case.</div><div cla= ss=3D"gmail_attr"><br></div><div class=3D"gmail_attr">Please, lets' do = this in the header with a static inline like I suggested to get around this= issue.</div><div class=3D"gmail_attr"><br></div><div class=3D"gmail_attr">= Warner<br></div><div dir=3D"ltr" class=3D"gmail_attr"><br></div><div dir=3D= "ltr" class=3D"gmail_attr">On Tue, Jul 18, 2023 at 7:20=E2=80=AFPM John Bal= dwin <<a href=3D"mailto:jhb@freebsd.org">jhb@freebsd.org</a>> wrote:<= br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8e= x;border-left:1px solid rgb(204,204,204);padding-left:1ex">The branch main = has been updated by jhb:<br> <br> URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3Dc5312bd79e66ce8ef= 50655ce7f3eca06d6b6e24f" rel=3D"noreferrer" target=3D"_blank">https://cgit.= FreeBSD.org/src/commit/?id=3Dc5312bd79e66ce8ef50655ce7f3eca06d6b6e24f</a><b= r> <br> commit c5312bd79e66ce8ef50655ce7f3eca06d6b6e24f<br> Author:=C2=A0 =C2=A0 =C2=A0John Baldwin <jhb@FreeBSD.org><br> AuthorDate: 2023-07-19 01:19:27 +0000<br> Commit:=C2=A0 =C2=A0 =C2=A0John Baldwin <jhb@FreeBSD.org><br> CommitDate: 2023-07-19 01:19:27 +0000<br> <br> =C2=A0 =C2=A0 cam: Move bus_dmamap_load_ccb into cam.c.<br> <br> =C2=A0 =C2=A0 This routine is specific to CAM and no longer assumes any int= ernal<br> =C2=A0 =C2=A0 bus_dma knowledge as it is simple wrapper around bus_dmamap_l= oad_mem.<br> <br> =C2=A0 =C2=A0 Fixes:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 60381fd1ee86 memdesc= : Retire MEMDESC_CCB.<br> =C2=A0 =C2=A0 Reviewed by:=C2=A0 =C2=A0 kib<br> =C2=A0 =C2=A0 Differential Revision:=C2=A0 <a href=3D"https://reviews.freeb= sd.org/D41058" rel=3D"noreferrer" target=3D"_blank">https://reviews.freebsd= .org/D41058</a><br> ---<br> =C2=A0sys/cam/cam.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 20 ++++++++++= ++++++++++<br> =C2=A0sys/kern/subr_bus_dma.c | 19 -------------------<br> =C2=A02 files changed, 20 insertions(+), 19 deletions(-)<br> <br> diff --git a/sys/cam/cam.c b/sys/cam/cam.c<br> index ce7dc81b3495..7d9d8602d009 100644<br> --- a/sys/cam/cam.c<br> +++ b/sys/cam/cam.c<br> @@ -52,6 +52,7 @@ __FBSDID("$FreeBSD$");<br> <br> =C2=A0#ifdef _KERNEL<br> =C2=A0#include <sys/libkern.h><br> +#include <machine/bus.h><br> =C2=A0#include <cam/cam_queue.h><br> =C2=A0#include <cam/cam_xpt.h><br> <br> @@ -642,4 +643,23 @@ memdesc_ccb(union ccb *ccb)<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 panic("%s: fla= gs 0x%X unimplemented", __func__, ccb_h->flags);<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 }<br> =C2=A0}<br> +<br> +int<br> +bus_dmamap_load_ccb(bus_dma_tag_t dmat, bus_dmamap_t map, union ccb *ccb,<= br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0bus_d= mamap_callback_t *callback, void *callback_arg,<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int f= lags)<br> +{<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0struct ccb_hdr *ccb_h;<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0struct memdesc mem;<br> +<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0ccb_h =3D &ccb->ccb_h;<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0if ((ccb_h->flags & CAM_DIR_MASK) =3D=3D= CAM_DIR_NONE) {<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0callback(callback_a= rg, NULL, 0, 0);<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return (0);<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> +<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0mem =3D memdesc_ccb(ccb);<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0return (bus_dmamap_load_mem(dmat, map, &mem= , callback, callback_arg,<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0flags));<br> +}<br> =C2=A0#endif<br> diff --git a/sys/kern/subr_bus_dma.c b/sys/kern/subr_bus_dma.c<br> index da7a2ee4cdc9..683b41d0047c 100644<br> --- a/sys/kern/subr_bus_dma.c<br> +++ b/sys/kern/subr_bus_dma.c<br> @@ -449,25 +449,6 @@ bus_dmamap_load_uio(bus_dma_tag_t dmat, bus_dmamap_t m= ap, struct uio *uio,<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (error);<br> =C2=A0}<br> <br> -int<br> -bus_dmamap_load_ccb(bus_dma_tag_t dmat, bus_dmamap_t map, union ccb *ccb,<= br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0bus_d= mamap_callback_t *callback, void *callback_arg,<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int f= lags)<br> -{<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0struct ccb_hdr *ccb_h;<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0struct memdesc mem;<br> -<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0ccb_h =3D &ccb->ccb_h;<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0if ((ccb_h->flags & CAM_DIR_MASK) =3D=3D= CAM_DIR_NONE) {<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0callback(callback_a= rg, NULL, 0, 0);<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return (0);<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> -<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0mem =3D memdesc_ccb(ccb);<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0return (bus_dmamap_load_mem(dmat, map, &mem= , callback, callback_arg,<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0flags));<br> -}<br> -<br> =C2=A0int<br> =C2=A0bus_dmamap_load_bio(bus_dma_tag_t dmat, bus_dmamap_t map, struct bio = *bio,<br> =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,<br> </blockquote></div></div> --00000000000075cc400600cd2ca2--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfotvSACUJshOLDR-qd97z1BTwgeYqtYrFYW6-_wSzL1CQ>