Re: [PATCH v1] drm: import DMA_BUF module namespace

[Marcel Ziswiler]

On Thu, 2021-10-28 at 20:45 +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 27.10.21 um 22:54 schrieb Marcel Ziswiler:
> > Sali Thomas
> > 
> > On Wed, 2021-10-27 at 20:30 +0200, Thomas Zimmermann wrote:
> > > Hi,
> > > 
> > > thanks for the patch.
> > 
> > You are very welcome.
> > 
> > > Am 27.10.21 um 17:25 schrieb Marcel Ziswiler:
> > > > From: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx>
> > > > 
> > > > Today's -next fails building arm64 defconfig as follows:
> > > > 
> > > > ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vunmap from
> > > >    namespace DMA_BUF, but does not import it.
> > > > ERROR: modpost: module drm_cma_helper uses symbol dma_buf_vmap from
> > > >    namespace DMA_BUF, but does not import it.
> > > > 
> > > > Reported-by: Linux Kernel Functional Testing <lkft@xxxxxxxxxx>
> > > > Fixes: commit 4b2b5e142ff4 ("drm: Move GEM memory managers into modules")
> > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx>
> > > > 
> > > > ---
> > > > 
> > > >    drivers/gpu/drm/drm_gem_cma_helper.c | 2 ++
> > > >    1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > index 6f7b3f8ec04d3..69f8564ad11cd 100644
> > > > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > @@ -23,6 +23,8 @@
> > > >    #include <drm/drm_gem_cma_helper.h>
> > > >    #include <drm/drm_vma_manager.h>
> > > >    
> > > > +MODULE_IMPORT_NS(DMA_BUF);
> > > 
> > > Could this line be moved to the bottom of the file, where the other
> > > MODULE statements are?
> > 
> > Hehe, good question. I was actually asking myself the same but quickly looking at a few files and they all
> > had
> > it after their includes towards the top. Turns out that was rather short sighted...
> > 
> > Let me look more closely. Current -next has exactly 200 files with a MODULE_IMPORT_NS statement. Okay, some
> > of
> > which are documentation. Anyway, 132 of which do have it with their other MODULE macros towards the end as
> > you
> > suggest. 20 of which and mainly DRM stuff has it towards the top after them includes. Funny.
> > 
> > What does the documentation suggest?
> > 
> > Documentation/core-api/symbol-namespaces.rst
> > 
> > "It is advisable to add the MODULE_IMPORT_NS() statement close to other module
> > metadata definitions like MODULE_AUTHOR() or MODULE_LICENSE(). Refer to section
> > 5. for a way to create missing import statements automatically."
> > 
> > There you go. Plus there is even some fancy automation (;-p).
> > 
> > So let me move it down there then.
> 
> Will you send out another revision of the patch?

I already did.

https://marc.info/?l=linux-arm-kernel&m=163537686316807

> > > In the fixed commit 4b2b5e142ff4, there's a similar change for
> > > drm_gem_shmem_helper.c. It uses dma-buf_vmap as well. Does that module
> > > require the same fix?
> > 
> > Likely. Let me just run ze automation and see what we get...
> > 
> > > Do you have any idea why I don't see these errors in my builds?
> > 
> > Well, I guess, there are various KCONFIG symbols influencing that whole story. How about e.g.
> > 
> > init/Kconfig:config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
> 
> Thanks for the hint.

Welcome.

> Best regards
> Thomas
> 
> > 
> > > Best regards
> > > Thomas
> > 
> > Cheers
> > 
> > Marcel
> > 
> > > > +
> > > >    /**
> > > >     * DOC: cma helpers
> > > >     *
> > > > 
> > > 
> > > -- 
> > > Thomas Zimmermann
> > > Graphics Driver Developer
> > > SUSE Software Solutions Germany GmbH
> > > Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > (HRB 36809, AG Nürnberg)
> > > Geschäftsführer: Felix Imendörffer
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer