linux-kernel - Re: [PATCH v11 1/7] ARM: probes: move all probe code to dedicate directory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 3 Dec 2014 13:28:41 +0800
From:	Wang Nan <wangnan0@...wei.com>
To:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
CC:	<tixy@...aro.org>, <linux@....linux.org.uk>, <lizefan@...wei.com>,
	<linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v11 1/7] ARM: probes: move all probe code to dedicate
 directory

On 2014/12/3 12:38, Masami Hiramatsu wrote:
> (2014/12/02 19:23), Wang Nan wrote:
>> On 2014/12/2 12:59, Masami Hiramatsu wrote:
>>> (2014/12/01 17:48), Wang Nan wrote:
>>>> In discussion on LKML (https://lkml.org/lkml/2014/11/28/158), Russell
>>>> King suggest to move all probe related code to arch/arm/probes. This
>>>> patch do the work. Due to dependency on 'arch/arm/kernel/patch.h', this
>>>> patch also move patch.h to 'arch/arm/include/asm/patch.h', and related
>>>> '#include' directive are also midified to '#include <asm/patch.h>'.
>>>
>>> Moving is good to me, but renaming files are also required I think.
>>>
>>>> Signed-off-by: Wang Nan <wangnan0@...wei.com>
>>>> ---
>>>>  arch/arm/Makefile                                |  1 +
>>>>  arch/arm/{kernel => include/asm}/patch.h         |  0
>>>>  arch/arm/kernel/Makefile                         | 16 ++--------------
>>>>  arch/arm/kernel/jump_label.c                     |  2 +-
>>>>  arch/arm/kernel/patch.c                          |  3 +--
>>>>  arch/arm/probes/Makefile                         | 15 +++++++++++++++
>>>>  arch/arm/{kernel => probes}/kprobes-arm.c        |  0
>>>>  arch/arm/{kernel => probes}/kprobes-common.c     |  0
>>>>  arch/arm/{kernel => probes}/kprobes-test-arm.c   |  0
>>>>  arch/arm/{kernel => probes}/kprobes-test-thumb.c |  0
>>>>  arch/arm/{kernel => probes}/kprobes-test.c       |  0
>>>>  arch/arm/{kernel => probes}/kprobes-test.h       |  0
>>>>  arch/arm/{kernel => probes}/kprobes-thumb.c      |  0
>>>>  arch/arm/{kernel => probes}/kprobes.c            |  2 +-
>>>>  arch/arm/{kernel => probes}/kprobes.h            |  0
>>>>  arch/arm/{kernel => probes}/probes-arm.c         |  0
>>>>  arch/arm/{kernel => probes}/probes-arm.h         |  0
>>>>  arch/arm/{kernel => probes}/probes-thumb.c       |  0
>>>>  arch/arm/{kernel => probes}/probes-thumb.h       |  0
>>>>  arch/arm/{kernel => probes}/probes.c             |  0
>>>>  arch/arm/{kernel => probes}/probes.h             |  0
>>>>  arch/arm/{kernel => probes}/uprobes-arm.c        |  0
>>>>  arch/arm/{kernel => probes}/uprobes.c            |  0
>>>>  arch/arm/{kernel => probes}/uprobes.h            |  0
>>>
>>> As I did on x86, these would be better renamed as expressing what they do.
>>> I guess most of the files may have emulate-*.c or decode-*.c :)
>>>
>>> Thank you,
>>>
>>
>> OK. I posted another patch in this thread. The directory tree is as follow:
>>
>> arch/arm/probes/
>> |-- Makefile
>> |-- decode-arm.c
>> |-- decode-arm.h
>> |-- decode-thumb.c
>> |-- decode-thumb.h
>> |-- decode.c
>> |-- decode.h
>> |-- kprobes
>> |   |-- actions-arm.c
>> |   |-- actions-common.c
>> |   |-- actions-thumb.c
>> |   |-- kprobes.c
>> |   |-- kprobes.h
>> |   |-- test-arm.c
>> |   |-- test-core.c
>> |   |-- test-core.h
>> |   `-- test-thumb.c
>> `-- uprobes
>>     |-- actions-arm.c
>>     |-- uprobes.c
>>     `-- uprobes.h
>>
>> 2 directories, 19 files
>>
>> What do you think?
> 
> Yeah, that looks better :)
> 
> Btw, if you introduce probes/{kprobes,uprobes}/, *probes.c should be core.c too,
> since the directories already show its name. And also, both dirs should have its
> own Makefile.
> 

Seprated Makefile may introduce extra complexity. Think about someone try to compile
kprobe as a module (currently it is impossible due to dependencies between kprobe and
kernel core, but decoupling is possible), seprated Makefiles may force him to
create at least 3 modules for kprobe, even if one module is enough.

Anyway, there may some features in kernel build system can help him. I'll post
v12 patch series based on your suggestion.

Thank you.

> Thank you,
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/