Re: [PATCH 2/2] memory_driver: Support overriding kernel directory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Lianbo,

Thanks for testing! My response is inline below...

On 27.09.23 11:14, lijiang wrote:
> Hi, Mathias
> Thank you for the patchset.
> On Tue, Sep 26, 2023 at 10:36 PM <crash-utility-request@xxxxxxxxxx
> <mailto:crash-utility-request@xxxxxxxxxx>> wrote:
> 
>     Date: Tue, 26 Sep 2023 12:12:47 +0200
>     From: Mathias Krause <minipli@xxxxxxxxxxxxxx
>     <mailto:minipli@xxxxxxxxxxxxxx>>
>     To: crash-utility@xxxxxxxxxx <mailto:crash-utility@xxxxxxxxxx>
>     Subject:  [PATCH 2/2] memory_driver: Support overriding
>             kernel directory
>     Message-ID: <20230926101247.1237748-3-minipli@xxxxxxxxxxxxxx
>     <mailto:20230926101247.1237748-3-minipli@xxxxxxxxxxxxxx>>
>     Content-Type: text/plain; charset="US-ASCII"; x-default=true
> 
>     Support compiling the module against a different kernel version than the
>     currently running one by allowing to set either KVER or KDIR variables
>     on the make commandline.
> 
>     Also modernize the makefile slightly and make use of the kernel's
>     'clean' target to ensure to remove all generated files.
> 
> 
> The [PATCH 1/2] looks good to me.

Thanks.

> 
> For the [PATCH 2/2], I only have two questions:
> 
> [1] With the patch 2/2, it always triggers recompiling the gdb like this:
> # make lzo
> TARGET: PPC64
>  CRASH: 8.0.3++
>    GDB: 10.2
> 
>   CXX    gdb.o
>   CXX    ../../crash_target.o
>   CXX    ada-exp.o
>   CXX    ada-lang.o
>   CXX    ada-tasks.o
>   CXX    ada-typeprint.o
>   CXX    ada-valprint.o
>   CXX    ada-varobj.o
>   CXX    addrmap.o
>   CXX    agent.o
>   CXX    alloc.o
>   CXX    annotate.o
> ...

Hmm, I cannot reproduce this. If I do 'make lzo' it builds 'crash' only
once, not multiple times as you're observing. Can you please provide
instructions how to reproduce the issue? Or, maybe, it's related to the
second one...

> 
> [2] With the patch 2/2, it always reports the following error "No such
> file or directory", if the kernel-devel package is not installed.
> # make clean
> ...
> make -C /lib/modules/xxx/build M=/home/crash SUBDIRS=/home/crash clean
> || rm -f *.mod.c *.ko *.o Module.*
> make[3]: *** /lib/modules/xxx/build: No such file or directory.  Stop.
> 
> Actually, I did not build the crash.ko in the directory memory_driver/.
> 
> Is that expected behavior?

Ahh, yes. That's expected, but unfortunate behavior. I'll fix it by
testing if the build directory exists first to avoid the error message.

The fall-back handling is already there -- simply does the old 'rm
*.mod.c *.ko *.o Module.*'. However, there's no need to generate an
error message when we know in advance that the kernel build directory is
missing.

I'll send v2 in a moment.

Thanks,
Mathias

> 
> Thanks
> Lianbo
>  
> 
>     Signed-off-by: Mathias Krause <minipli@xxxxxxxxxxxxxx
>     <mailto:minipli@xxxxxxxxxxxxxx>>
>     ---
>      memory_driver/Makefile | 9 +++++++--
>      1 file changed, 7 insertions(+), 2 deletions(-)
> 
>     diff --git a/memory_driver/Makefile b/memory_driver/Makefile
>     index b494aa3cd184..b720661fa75f 100644
>     --- a/memory_driver/Makefile
>     +++ b/memory_driver/Makefile
>     @@ -8,8 +8,13 @@
>      # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>      # GNU General Public License for more details.
>      #
>     +ifneq ($(KERNELRELEASE),)
>      obj-m := crash.o
>     +else
>     +KVER ?= $(shell uname -r)
>     +KDIR ?= /lib/modules/${KVER}/build
>      all:
>     -       make -C /lib/modules/`uname -r`/build M=${PWD}
>     SUBDIRS=${PWD} modules
>     +       ${MAKE} -C ${KDIR} M=${PWD} SUBDIRS=${PWD} modules
>      clean:
>     -       rm -f *.mod.c *.ko *.o Module.*
>     +       ${MAKE} -C ${KDIR} M=${PWD} SUBDIRS=${PWD} clean || ${RM}
>     *.mod.c *.ko *.o Module.*
>     +endif
>     -- 
>     2.30.2
> 

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux