[1/2] kcmp: Make it to depend on CONFIG_KCMP

Message ID	20130219182838.GL20312@moon (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org> Date: Tue, 19 Feb 2013 22:28:38 +0400 From: Cyrill Gorcunov <gorcunov@openvz.org> To: "H. Peter Anvin" <hpa@zytor.com> Subject: Re: [patch 1/2] kcmp: Make it to depend on CONFIG_KCMP Message-ID: <20130219182838.GL20312@moon> References: <20130219064800.719149796@openvz.org> <20130219065210.030802820@openvz.org> <5123444D.6000806@suse.cz> <20130219093154.GF20312@moon> <5123BC2B.6000304@zytor.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5123BC2B.6000304@zytor.com> User-Agent: Mutt/1.5.21 (2010-09-15) summary: Content analysis details: (1.1 points) pts rule name description ---- ---------------------- -------------------------------------------------- 3.0 KHOP_BIG_TO_CC Sent to 10+ recipients instaed of Bcc or a list 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (gorcunov[at]gmail.com) -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: Michal Marek <mmarek@suse.cz>, Andrey Vagin <avagin@openvz.org>, Eric Dumazet <eric.dumazet@gmail.com>, xemul@parallels.com, Alexey Dobriyan <adobriyan@gmail.com>, Frederic Weisbecker <fweisbec@gmail.com>, Vasiliy Kulikov <segoon@openwall.com>, linux-kernel@vger.kernel.org, Glauber Costa <glommer@parallels.com>, Pekka Enberg <penberg@kernel.org>, Andi Kleen <andi@firstfloor.org>, Thomas Gleixner <tglx@linutronix.de>, ebiederm@xmission.com, Tejun Heo <tj@kernel.org>, KOSAKI Motohiro <kosaki.motohiro@gmail.com>, akpm@linux-foundation.org, Ingo Molnar <mingo@elte.hu>, linux-arm-kernel@lists.infradead.org, Matt Helsley <matthltc@us.ibm.com> Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org

Cyrill Gorcunov Feb. 19, 2013, 6:28 p.m. UTC

On Tue, Feb 19, 2013 at 09:53:47AM -0800, H. Peter Anvin wrote:
> On 02/19/2013 01:31 AM, Cyrill Gorcunov wrote:
> >+
> >+	  If unsure, say N.
> >+
> 
> Wrong advice.  In this particular case, Y is the safe alternative.
---
From: Cyrill Gorcunov <gorcunov@openvz.org>
Subject: kcmp: Make it to depend on CONFIG_KCMP

Since kcmp syscall has been implemented (initially on
x86 architecture) a number of other archs wire it up
as well: xtensa, sparc, sh, s390, mips, microblaze,
m68k (not taking into account those who uses
<asm-generic/unistd.h> for syscall numbers
definitions).

But the Makefile, which turns kcmp.o generation on
still depends on former config-x86. Thus get rid
of this limitation and make kcmp.o depend on CONFIG_KCMP
option.

v2:
 - As Michal pointed the old configs might already use of
   CHECKPOINT_RESTORE, so make "default" accordingly.
 - Advice Y if unsure

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Matt Helsley <matthltc@us.ibm.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Valdis.Kletnieks@vt.edu
Cc: Michal Marek <mmarek@suse.cz>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 init/Kconfig    |    9 +++++++++
 kernel/Makefile |    4 +---
 2 files changed, 10 insertions(+), 3 deletions(-)

Andrew Morton Feb. 19, 2013, 9:42 p.m. UTC | #1

On Tue, 19 Feb 2013 22:28:38 +0400
Cyrill Gorcunov <gorcunov@openvz.org> wrote:

> Since kcmp syscall has been implemented (initially on
> x86 architecture) a number of other archs wire it up
> as well: xtensa, sparc, sh, s390, mips, microblaze,
> m68k (not taking into account those who uses
> <asm-generic/unistd.h> for syscall numbers
> definitions).
> 
> But the Makefile, which turns kcmp.o generation on
> still depends on former config-x86. Thus get rid
> of this limitation and make kcmp.o depend on CONFIG_KCMP
> option.
> 
> ...
>
> --- linux-2.6.git.orig/init/Kconfig
> +++ linux-2.6.git/init/Kconfig
> @@ -279,6 +279,15 @@ config FHANDLE
>  	  get renamed. Enables open_by_handle_at(2) and name_to_handle_at(2)
>  	  syscalls.
>  
> +config KCMP
> +	bool "kcmp syscall"
> +	default CHECKPOINT_RESTORE
> +	help
> +	  If you say Y here, a user level program will be able to use
> +	  kcmp(2) syscall.
> +
> +	  If unsure, say Y.
> +
>  config AUDIT
>  	bool "Auditing support"
>  	depends on NET
> Index: linux-2.6.git/kernel/Makefile
> ===================================================================
> --- linux-2.6.git.orig/kernel/Makefile
> +++ linux-2.6.git/kernel/Makefile
> @@ -25,9 +25,7 @@ endif
>  obj-y += sched/
>  obj-y += power/
>  
> -ifeq ($(CONFIG_CHECKPOINT_RESTORE),y)
> -obj-$(CONFIG_X86) += kcmp.o
> -endif
> +obj-$(CONFIG_KCMP) += kcmp.o
>  obj-$(CONFIG_FREEZER) += freezer.o
>  obj-$(CONFIG_PROFILING) += profile.o
>  obj-$(CONFIG_STACKTRACE) += stacktrace.o

This permits people to select kcmp with CONFIG_CHECKPOINT_RESTORE=n. 
Is there any point in doing that?

What would be wrong with just doing

	obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o

?

H. Peter Anvin Feb. 19, 2013, 9:48 p.m. UTC | #2

On 02/19/2013 01:42 PM, Andrew Morton wrote:
>> Index: linux-2.6.git/kernel/Makefile
>> ===================================================================
>> --- linux-2.6.git.orig/kernel/Makefile
>> +++ linux-2.6.git/kernel/Makefile
>> @@ -25,9 +25,7 @@ endif
>>   obj-y += sched/
>>   obj-y += power/
>>
>> -ifeq ($(CONFIG_CHECKPOINT_RESTORE),y)
>> -obj-$(CONFIG_X86) += kcmp.o
>> -endif
>> +obj-$(CONFIG_KCMP) += kcmp.o
>>   obj-$(CONFIG_FREEZER) += freezer.o
>>   obj-$(CONFIG_PROFILING) += profile.o
>>   obj-$(CONFIG_STACKTRACE) += stacktrace.o
>
> This permits people to select kcmp with CONFIG_CHECKPOINT_RESTORE=n.
> Is there any point in doing that?
>
> What would be wrong with just doing
>
> 	obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o
>

The real question is if there are any potential use cases of kcmp() 
outside checkpoint/restore.  It is actually a very general facility.

	-hpa

Cyrill Gorcunov Feb. 19, 2013, 9:54 p.m. UTC | #3

On Tue, Feb 19, 2013 at 01:42:56PM -0800, Andrew Morton wrote:
> > Index: linux-2.6.git/kernel/Makefile
> > ===================================================================
> > --- linux-2.6.git.orig/kernel/Makefile
> > +++ linux-2.6.git/kernel/Makefile
> > @@ -25,9 +25,7 @@ endif
> >  obj-y += sched/
> >  obj-y += power/
> >  
> > -ifeq ($(CONFIG_CHECKPOINT_RESTORE),y)
> > -obj-$(CONFIG_X86) += kcmp.o
> > -endif
> > +obj-$(CONFIG_KCMP) += kcmp.o
> >  obj-$(CONFIG_FREEZER) += freezer.o
> >  obj-$(CONFIG_PROFILING) += profile.o
> >  obj-$(CONFIG_STACKTRACE) += stacktrace.o
> 
> This permits people to select kcmp with CONFIG_CHECKPOINT_RESTORE=n. 
> Is there any point in doing that?
> 
> What would be wrong with just doing
> 
> 	obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o

I think this syscall is usefull even without c/r stuff.
That's why I made it with separate config option.

Andrew Morton Feb. 19, 2013, 10 p.m. UTC | #4

On Wed, 20 Feb 2013 01:54:32 +0400
Cyrill Gorcunov <gorcunov@openvz.org> wrote:

> On Tue, Feb 19, 2013 at 01:42:56PM -0800, Andrew Morton wrote:
> > > Index: linux-2.6.git/kernel/Makefile
> > > ===================================================================
> > > --- linux-2.6.git.orig/kernel/Makefile
> > > +++ linux-2.6.git/kernel/Makefile
> > > @@ -25,9 +25,7 @@ endif
> > >  obj-y += sched/
> > >  obj-y += power/
> > >  
> > > -ifeq ($(CONFIG_CHECKPOINT_RESTORE),y)
> > > -obj-$(CONFIG_X86) += kcmp.o
> > > -endif
> > > +obj-$(CONFIG_KCMP) += kcmp.o
> > >  obj-$(CONFIG_FREEZER) += freezer.o
> > >  obj-$(CONFIG_PROFILING) += profile.o
> > >  obj-$(CONFIG_STACKTRACE) += stacktrace.o
> > 
> > This permits people to select kcmp with CONFIG_CHECKPOINT_RESTORE=n. 
> > Is there any point in doing that?
> > 
> > What would be wrong with just doing
> > 
> > 	obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o
> 
> I think this syscall is usefull even without c/r stuff.
> That's why I made it with separate config option.

hm, OK.

But the patch also permits CONFIG_CHECKPOINT_RESTORE=y, CONFIG_KCMP=n
which surely isn't something which CRIU wants to support?

Cyrill Gorcunov Feb. 19, 2013, 10:02 p.m. UTC | #5

On Tue, Feb 19, 2013 at 01:48:27PM -0800, H. Peter Anvin wrote:
> >
> >This permits people to select kcmp with CONFIG_CHECKPOINT_RESTORE=n.
> >Is there any point in doing that?
> >
> >What would be wrong with just doing
> >
> >	obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o
> >
> 
> The real question is if there are any potential use cases of kcmp()
> outside checkpoint/restore.  It is actually a very general facility.

As far as I remember Eric was pointing to such potential use at early
time when kcmp being only started developing (I'm trying to find this
email in archive, if only my memory doesn't betray me and it was about
something else ;) Nevertheless, one may write utility to output
statistics on shared "resource" used by a task (don't know if it's
that useful since we use kcmp for c/r sake, but still).

Cyrill Gorcunov Feb. 19, 2013, 10:11 p.m. UTC | #6

On Tue, Feb 19, 2013 at 02:00:35PM -0800, Andrew Morton wrote:
> > 
> > I think this syscall is usefull even without c/r stuff.
> > That's why I made it with separate config option.
> 
> hm, OK.
> 
> But the patch also permits CONFIG_CHECKPOINT_RESTORE=y, CONFIG_KCMP=n
> which surely isn't something which CRIU wants to support?

Hmm, yes from one pov this feature is useful for out-of-c/r user,
from another pov -- we will have to ask users to turn on additional
CONFIG entries (which i'm sure not set by default in wide range of distros).
Thus it seems less paiful way is either make it obj-(CHECKPOINT_RESTORE)
as you proposed, or obj-y by default. The last can't be undone, so I'll
prepare the patch for obj-(CHECKPOINT_RESTORE) I think.

H. Peter Anvin Feb. 19, 2013, 11:41 p.m. UTC | #7

On 02/19/2013 02:11 PM, Cyrill Gorcunov wrote:
> On Tue, Feb 19, 2013 at 02:00:35PM -0800, Andrew Morton wrote:
>>>
>>> I think this syscall is usefull even without c/r stuff.
>>> That's why I made it with separate config option.
>>
>> hm, OK.
>>
>> But the patch also permits CONFIG_CHECKPOINT_RESTORE=y, CONFIG_KCMP=n
>> which surely isn't something which CRIU wants to support?
>
> Hmm, yes from one pov this feature is useful for out-of-c/r user,
> from another pov -- we will have to ask users to turn on additional
> CONFIG entries (which i'm sure not set by default in wide range of distros).
> Thus it seems less paiful way is either make it obj-(CHECKPOINT_RESTORE)
> as you proposed, or obj-y by default. The last can't be undone, so I'll
> prepare the patch for obj-(CHECKPOINT_RESTORE) I think.
>

Well, that's what dependencies are for.

Either way we might just want to wait for such a use case to appear, I 
don't know.

	-hpa

[1/2] kcmp: Make it to depend on CONFIG_KCMP

Commit Message

Comments

Patch