On Mon, Jul 17, 2023 at 11:54 AM Konstantin Belousov <[email protected]> wrote:
> On Mon, Jul 17, 2023 at 11:29:20AM -0600, Warner Losh wrote: > > On Mon, Jul 17, 2023 at 11:15 AM John Baldwin <[email protected]> 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=60381fd1ee8668ea1e4676a6128883d987cab858 > > > >> > > > >> commit 60381fd1ee8668ea1e4676a6128883d987cab858 > > > >> Author: John Baldwin <[email protected]> > > > >> AuthorDate: 2023-07-14 18:30:31 +0000 > > > >> Commit: John Baldwin <[email protected]> > > > >> 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 = 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
