All of lore.kernel.org
 help / color / mirror / Atom feed
From: pmladek@suse.com (Petr Mladek)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 6/7] arm: Add livepatch to build if CONFIG_LIVEPATCH
Date: Wed, 7 Dec 2016 16:05:25 +0100	[thread overview]
Message-ID: <20161207150525.GD13288@linux.suse> (raw)
In-Reply-To: <1481043967-15602-7-git-send-email-abelvesa@linux.com>

On Tue 2016-12-06 17:06:06, Abel Vesa wrote:
> Necessary livepatch file added to makefile.
> 
> Signed-off-by: Abel Vesa <abelvesa@linux.com>
> ---
>  arch/arm/kernel/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index ad325a8..9e70220 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
>  obj-$(CONFIG_ARM_ARCH_TIMER)	+= arch_timer.o
>  obj-$(CONFIG_FUNCTION_TRACER)	+= entry-ftrace.o
>  obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o insn.o
> +obj-$(CONFIG_LIVEPATCH)		+= livepatch.o

It is strange that you add a source file in one patch and make it
build in a much later patch.

I suggest to restructure the entire patchset a bit. Please, first
add support for FTRACE_WITH_REGS. It makes sense on its own.
Then add the livepatch support on top of it.

Otherwise, it is not necessary to send v2 immediately for such
non-trivial code. There might be more people that would want
to look at it and it might take days until they find a time.
It is always better to collect some feedback, think about it
over night(s). Every question often opens many other questions
and it usually takes some time until all settles down into
a good picture again.

Best Regards,
Petr

WARNING: multiple messages have this Message-ID (diff)
From: Petr Mladek <pmladek@suse.com>
To: Abel Vesa <abelvesa@linux.com>
Cc: linux@armlinux.org.uk, jpoimboe@redhat.com, jeyu@redhat.com,
	jikos@kernel.org, mbenes@suse.cz, rostedt@goodmis.org,
	mingo@redhat.com, gregkh@linuxfoundation.org,
	geert+renesas@glider.be, davem@davemloft.net,
	akpm@linux-foundation.org, emil.l.velikov@gmail.com,
	mchehab@kernel.org, linux@roeck-us.net,
	ard.biesheuvel@linaro.org, jens.wiklander@linaro.org,
	jean-philippe.brucker@arm.com, viro@zeniv.linux.org.uk,
	stefano.stabellini@eu.citrix.com, chris.brandt@renesas.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	live-patching@vger.kernel.org
Subject: Re: [PATCH 6/7] arm: Add livepatch to build if CONFIG_LIVEPATCH
Date: Wed, 7 Dec 2016 16:05:25 +0100	[thread overview]
Message-ID: <20161207150525.GD13288@linux.suse> (raw)
In-Reply-To: <1481043967-15602-7-git-send-email-abelvesa@linux.com>

On Tue 2016-12-06 17:06:06, Abel Vesa wrote:
> Necessary livepatch file added to makefile.
> 
> Signed-off-by: Abel Vesa <abelvesa@linux.com>
> ---
>  arch/arm/kernel/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index ad325a8..9e70220 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
>  obj-$(CONFIG_ARM_ARCH_TIMER)	+= arch_timer.o
>  obj-$(CONFIG_FUNCTION_TRACER)	+= entry-ftrace.o
>  obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o insn.o
> +obj-$(CONFIG_LIVEPATCH)		+= livepatch.o

It is strange that you add a source file in one patch and make it
build in a much later patch.

I suggest to restructure the entire patchset a bit. Please, first
add support for FTRACE_WITH_REGS. It makes sense on its own.
Then add the livepatch support on top of it.

Otherwise, it is not necessary to send v2 immediately for such
non-trivial code. There might be more people that would want
to look at it and it might take days until they find a time.
It is always better to collect some feedback, think about it
over night(s). Every question often opens many other questions
and it usually takes some time until all settles down into
a good picture again.

Best Regards,
Petr

  reply	other threads:[~2016-12-07 15:05 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-06 17:06 [PATCH 0/7] arm: Add livepatch support Abel Vesa
2016-12-06 17:06 ` Abel Vesa
2016-12-06 17:06 ` [PATCH 1/7] arm: Add livepatch arch specific code Abel Vesa
2016-12-06 17:06   ` Abel Vesa
2017-01-16 16:47   ` Miroslav Benes
2017-01-16 16:47     ` Miroslav Benes
2017-01-17  0:22     ` Jessica Yu
2017-01-17  0:22       ` Jessica Yu
2017-01-17  2:27       ` Jessica Yu
2017-01-17  2:27         ` Jessica Yu
2017-01-17 13:53       ` Miroslav Benes
2017-01-17 13:53         ` Miroslav Benes
2016-12-06 17:06 ` [PATCH 2/7] arm: ftrace: Add call modify mechanism Abel Vesa
2016-12-06 17:06   ` Abel Vesa
2016-12-07 10:37   ` kbuild test robot
2016-12-07 10:37     ` kbuild test robot
2016-12-06 17:06 ` [PATCH 3/7] arm: module: Add apply_relocate_add Abel Vesa
2016-12-06 17:06   ` Abel Vesa
2016-12-07  2:08   ` kbuild test robot
2016-12-07  2:08     ` kbuild test robot
2017-01-17  4:49   ` Jessica Yu
2017-01-17  4:49     ` Jessica Yu
2017-01-18 10:37     ` Miroslav Benes
2017-01-18 10:37       ` Miroslav Benes
2016-12-06 17:06 ` [PATCH 4/7] arm: Add ftrace with regs support Abel Vesa
2016-12-06 17:06   ` Abel Vesa
2016-12-07  2:43   ` Steven Rostedt
2016-12-07  2:43     ` Steven Rostedt
2016-12-07 10:57   ` Russell King - ARM Linux
2016-12-07 10:57     ` Russell King - ARM Linux
2016-12-07 11:58   ` Robin Murphy
2016-12-07 11:58     ` Robin Murphy
2016-12-07 12:10     ` Abel Vesa
2016-12-07 12:10       ` Abel Vesa
2016-12-06 17:06 ` [PATCH 5/7] arm: ftrace: Add ARCH_SUPPORTS_FTRACE_OPS for ftrace with regs Abel Vesa
2016-12-06 17:06   ` Abel Vesa
2016-12-06 17:06 ` [PATCH 6/7] arm: Add livepatch to build if CONFIG_LIVEPATCH Abel Vesa
2016-12-06 17:06   ` Abel Vesa
2016-12-07 15:05   ` Petr Mladek [this message]
2016-12-07 15:05     ` Petr Mladek
2016-12-07 16:11     ` Abel Vesa
2016-12-07 16:11       ` Abel Vesa
2017-01-18 12:36   ` Miroslav Benes
2017-01-18 12:36     ` Miroslav Benes
2016-12-06 17:06 ` [PATCH 7/7] arm: Add livepatch necessary arch selects into Kconfig Abel Vesa
2016-12-06 17:06   ` Abel Vesa
2016-12-07  2:45   ` Steven Rostedt
2016-12-07  2:45     ` Steven Rostedt
2016-12-07  6:48   ` kbuild test robot
2016-12-07  6:48     ` kbuild test robot
2017-01-18 12:40   ` Miroslav Benes
2017-01-18 12:40     ` Miroslav Benes
2017-01-18 13:35     ` Russell King - ARM Linux
2017-01-18 13:35       ` Russell King - ARM Linux
2016-12-07  1:35 ` [PATCH 0/7] arm: Add livepatch support zhouchengming
2016-12-07  1:38 ` zhouchengming
2016-12-07  1:38   ` zhouchengming
2016-12-07 11:39   ` Abel Vesa
2016-12-07 11:39     ` Abel Vesa
2016-12-07 15:19 ` Petr Mladek
2016-12-07 15:19   ` Petr Mladek

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=20161207150525.GD13288@linux.suse \
    --to=pmladek@suse.com \
    --cc=linux-arm-kernel@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.