Re: git: c5312bd79e66 - main - cam: Move bus_dmamap_load_ccb into cam.c.
- In reply to: Warner Losh : "Re: git: c5312bd79e66 - main - cam: Move bus_dmamap_load_ccb into cam.c."
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 19 Jul 2023 02:42:36 UTC
Sorry for the top post. Two ways forward that I see: Two proposed fixes https://reviews.freebsd.org/D41077 which makes loading the dma for a nvme request a function pointer (and keeps all ccb knowledge in nvme_sim.c) https://reviews.freebsd.org/D41078 which moves renames memdesc_ccb to cam_memdesc_ccb and moves it to cam_cch.h '78 is ready to commit as is. I think it's fine, but don't like the dependency graph. '77 likely needs some polish before it's pushed in, especially with naming. I like its dependency graph, but don't like the extra call through a function pointer for all nvme requests. Warner P.S. Sorry if I sounded too grumpy in other replies... it's been a frustrating day in $REAL_LIFE and I let that infect those replies. On Tue, Jul 18, 2023 at 7:44 PM Warner Losh <imp@bsdimp.com> wrote: > 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 PM John Baldwin <jhb@freebsd.org> wrote: > >> The branch main has been updated by jhb: >> >> URL: >> https://cgit.FreeBSD.org/src/commit/?id=c5312bd79e66ce8ef50655ce7f3eca06d6b6e24f >> >> 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 = &ccb->ccb_h; >> + if ((ccb_h->flags & CAM_DIR_MASK) == CAM_DIR_NONE) { >> + callback(callback_arg, NULL, 0, 0); >> + return (0); >> + } >> + >> + mem = 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 = &ccb->ccb_h; >> - if ((ccb_h->flags & CAM_DIR_MASK) == CAM_DIR_NONE) { >> - callback(callback_arg, NULL, 0, 0); >> - return (0); >> - } >> - >> - mem = 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 >> *bio, >> bus_dmamap_callback_t *callback, void *callback_arg, >> >