rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh@nvidia.com>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, cocci@inria.fr,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH v2 05/11] kbuild: change working directory to external module directory with M=
Date: Wed, 11 Dec 2024 12:21:27 +0000	[thread overview]
Message-ID: <b20db4db-afb4-4f58-bc01-ae1250abc8b0@nvidia.com> (raw)
In-Reply-To: <CAK7LNATgL=vOr37+XfF7du+_ak5yKgXYyNNNTEQdVvy5J2MMyA@mail.gmail.com>


On 11/12/2024 02:39, Masahiro Yamada wrote:
> On Wed, Dec 11, 2024 at 12:34 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> Hi Masahiro,
>>
>> On 10/11/2024 01:34, Masahiro Yamada wrote:
>>> Currently, Kbuild always operates in the output directory of the kernel,
>>> even when building external modules. This increases the risk of external
>>> module Makefiles attempting to write to the kernel directory.
>>>
>>> This commit switches the working directory to the external module
>>> directory, allowing the removal of the $(KBUILD_EXTMOD)/ prefix from
>>> some build artifacts.
>>>
>>> The command for building external modules maintains backward
>>> compatibility, but Makefiles that rely on working in the kernel
>>> directory may break. In such cases, $(objtree) and $(srctree) should
>>> be used to refer to the output and source directories of the kernel.
>>>
>>> The appearance of the build log will change as follows:
>>>
>>> [Before]
>>>
>>>     $ make -C /path/to/my/linux M=/path/to/my/externel/module
>>>     make: Entering directory '/path/to/my/linux'
>>>       CC [M]  /path/to/my/externel/module/helloworld.o
>>>       MODPOST /path/to/my/externel/module/Module.symvers
>>>       CC [M]  /path/to/my/externel/module/helloworld.mod.o
>>>       CC [M]  /path/to/my/externel/module/.module-common.o
>>>       LD [M]  /path/to/my/externel/module/helloworld.ko
>>>     make: Leaving directory '/path/to/my/linux'
>>>
>>> [After]
>>>
>>>     $ make -C /path/to/my/linux M=/path/to/my/externel/module
>>>     make: Entering directory '/path/to/my/linux'
>>>     make[1]: Entering directory '/path/to/my/externel/module'
>>>       CC [M]  helloworld.o
>>>       MODPOST Module.symvers
>>>       CC [M]  helloworld.mod.o
>>>       CC [M]  .module-common.o
>>>       LD [M]  helloworld.ko
>>>     make[1]: Leaving directory '/path/to/my/externel/module'
>>>     make: Leaving directory '/path/to/my/linux'
>>>
>>> Printing "Entering directory" twice is cumbersome. This will be
>>> addressed later.
>>>
>>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>>
>>
>> Since this change I have been observing the following build error when
>> building an external module ...
>>
>>    MODPOST Module.symvers
>> ERROR: modpost: drivers/gpu/host1x/host1x: 'host1x_device_init' exported
>>       twice. Previous export was in drivers/gpu/host1x/host1x.ko
>> ERROR: modpost: drivers/gpu/host1x/host1x: 'host1x_device_exit' exported
>>       twice. Previous export was in drivers/gpu/host1x/host1x.ko
>>
>> Now host1x is an upstream driver, but I have a local copy that using to
>> stage development changes (and avoid polluting the upstream driver).
>> Plus I can swap between which version I am using on a live system.
>>
>> What I noticed is that previously the Modules.symvers for the external
>> module had the full path of the external module for the name. However,
>> now the name is just the relative path and in this case
>> 'drivers/gpu/host1x/host1x'. Hence, this clashes with the in-kernel
>> driver and we get the 'exported twice' error.
>>
>> I have been looking to see if there is a way to fix this because it has
>> been a useful feature to override an upstream driver with a locally
>> modified version.
> 
> 
> I do not know how to reproduce it.
> 
>    if (s && (!external_module || s->module->is_vmlinux || s->module == mod)) {
> 
> is not checking the module path at all.
> I do not understand why it was affected.


So this is not explicitly checking the path, but comparing the contents
of the Module.symvers before and after this change for the external
module I see ...

$ grep -r host1x_device_init Module.symvers
0x00000000      host1x_device_init      /absolute/path/to/drivers/gpu/host1x/host1x        EXPORT_SYMBOL

And now I see ...

$ grep -r host1x_device_init Module.symvers
0x00000000      host1x_device_init      drivers/gpu/host1x/host1x  EXPORT_SYMBOL

So the problem is that now there is no longer an absolute path in the
external modules Module.symvers and so conflicts with the kernel's.

Does that make sense?

Jon

-- 
nvpublic


  reply	other threads:[~2024-12-11 12:21 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-10  1:34 [PATCH v2 00/11] kbuild: support building external modules in a separate build directory Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 01/11] kbuild: replace two $(abs_objtree) with $(CURDIR) in top Makefile Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 02/11] kbuild: add $(objtree)/ prefix to some in-kernel build artifacts Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 03/11] kbuild: rename abs_objtree to abs_output Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 04/11] kbuild: use 'output' variable to create the output directory Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 05/11] kbuild: change working directory to external module directory with M= Masahiro Yamada
2024-11-18 14:47   ` Nicolas Schier
2024-11-18 17:02     ` Masahiro Yamada
2024-11-27 15:29       ` Nicolas Schier
2024-11-27 23:15         ` Masahiro Yamada
2024-12-04 20:51   ` Alison Schofield
2024-12-05  2:33     ` Masahiro Yamada
2024-12-04 23:35   ` Charlie Jenkins
2024-12-05  2:48     ` Masahiro Yamada
2024-12-05  6:27       ` Charlie Jenkins
2024-12-09 13:46   ` Thorsten Blum
2024-12-09 13:55     ` Thorsten Blum
2024-12-10 10:47       ` Masahiro Yamada
2024-12-10 11:06         ` Thorsten Blum
2024-12-11  2:36           ` Masahiro Yamada
2024-12-10 15:34   ` Jon Hunter
2024-12-11  2:39     ` Masahiro Yamada
2024-12-11 12:21       ` Jon Hunter [this message]
2024-12-12  2:08         ` Masahiro Yamada
2024-12-12  6:00           ` Jon Hunter
2024-12-12 15:49             ` Masahiro Yamada
2025-01-27 23:08   ` Qu Wenruo
2024-11-10  1:34 ` [PATCH v2 06/11] kbuild: remove extmod_prefix, MODORDER, MODULES_NSDEPS variables Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 07/11] kbuild: support building external modules in a separate build directory Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 08/11] kbuild: support -fmacro-prefix-map for external modules Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 09/11] kbuild: use absolute path in the generated wrapper Makefile Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 10/11] kbuild: make wrapper Makefile more convenient for external modules Masahiro Yamada
2024-11-10  1:34 ` [PATCH v2 11/11] kbuild: allow to start building external modules in any directory Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b20db4db-afb4-4f58-bc01-ae1250abc8b0@nvidia.com \
    --to=jonathanh@nvidia.com \
    --cc=cocci@inria.fr \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).