Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1595006 > unrolled thread

[PATCH] livepatch: make klp_mutex proper part of API

Started byJiri Kosina <jikos@kernel.org>
First post2017-03-08 11:00 +0100
Last post2017-03-08 18:30 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] livepatch: make klp_mutex proper part of API Jiri Kosina <jikos@kernel.org> - 2017-03-08 11:00 +0100
    Re: [PATCH] livepatch: make klp_mutex proper part of API Miroslav Benes <mbenes@suse.cz> - 2017-03-08 13:30 +0100
    Re: [PATCH] livepatch: make klp_mutex proper part of API Miroslav Benes <mbenes@suse.cz> - 2017-03-08 14:20 +0100
    [PATCH v2] livepatch: make klp_mutex proper part of API Jiri Kosina <jikos@kernel.org> - 2017-03-08 14:30 +0100
      Re: [PATCH v2] livepatch: make klp_mutex proper part of API Miroslav Benes <mbenes@suse.cz> - 2017-03-08 14:40 +0100
      Re: [PATCH v2] livepatch: make klp_mutex proper part of API Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-08 14:40 +0100
    Re: [PATCH] livepatch: make klp_mutex proper part of API Josh Poimboeuf <jpoimboe@redhat.com> - 2017-03-08 18:30 +0100

#1595006 — [PATCH] livepatch: make klp_mutex proper part of API

FromJiri Kosina <jikos@kernel.org>
Date2017-03-08 11:00 +0100
Subject[PATCH] livepatch: make klp_mutex proper part of API
Message-ID<tiGxY-4My-29@gated-at.bofh.it>
From: Jiri Kosina <jkosina@suse.cz>

klp_mutex is shared between core.c and transition.c, and as such would 
rather be properly located in livepatch.h so that we don't have to play 
'extern' games from .c sources.

This also silences sparse warning (wrongly) suggesting that klp_mutex 
should be defined static.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 include/linux/livepatch.h     | 2 ++
 kernel/livepatch/transition.c | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 194991ef9347..4efa89f95dd7 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -138,6 +138,8 @@ struct klp_patch {
 	     func->old_name || func->new_func || func->old_sympos; \
 	     func++)
 
+extern struct mutex klp_mutex;
+
 int klp_register_patch(struct klp_patch *);
 int klp_unregister_patch(struct klp_patch *);
 int klp_enable_patch(struct klp_patch *);
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 0ab7abd53b0b..14f7ce64b002 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -28,8 +28,6 @@
 #define MAX_STACK_ENTRIES  100
 #define STACK_ERR_BUF_SIZE 128
 
-extern struct mutex klp_mutex;
-
 struct klp_patch *klp_transition_patch;
 
 static int klp_target_state = KLP_UNDEFINED;
-- 
2.11.0

[toc] | [next] | [standalone]

#1595115

FromMiroslav Benes <mbenes@suse.cz>
Date2017-03-08 13:30 +0100
Message-ID<tiIT8-6v2-27@gated-at.bofh.it>
In reply to#1595006
On Wed, 8 Mar 2017, Jiri Kosina wrote:

> From: Jiri Kosina <jkosina@suse.cz>
> 
> klp_mutex is shared between core.c and transition.c, and as such would 
> rather be properly located in livepatch.h so that we don't have to play 
> 'extern' games from .c sources.
> 
> This also silences sparse warning (wrongly) suggesting that klp_mutex 
> should be defined static.
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>  include/linux/livepatch.h     | 2 ++
>  kernel/livepatch/transition.c | 2 --
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 194991ef9347..4efa89f95dd7 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -138,6 +138,8 @@ struct klp_patch {
>  	     func->old_name || func->new_func || func->old_sympos; \
>  	     func++)
>  
> +extern struct mutex klp_mutex;
> +
>  int klp_register_patch(struct klp_patch *);
>  int klp_unregister_patch(struct klp_patch *);
>  int klp_enable_patch(struct klp_patch *);
> diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
> index 0ab7abd53b0b..14f7ce64b002 100644
> --- a/kernel/livepatch/transition.c
> +++ b/kernel/livepatch/transition.c
> @@ -28,8 +28,6 @@
>  #define MAX_STACK_ENTRIES  100
>  #define STACK_ERR_BUF_SIZE 128
>  
> -extern struct mutex klp_mutex;
> -

A nit, but could you also include "linux/livepatch.h" in transition.c to 
make the dependency explicit (and not through patch.h or transition.h)?

Anyway, not a big deal and you can add my

Acked-by: Miroslav Benes <mbenes@suse.cz>

Miroslav

[toc] | [prev] | [next] | [standalone]

#1595157

FromMiroslav Benes <mbenes@suse.cz>
Date2017-03-08 14:20 +0100
Message-ID<tiJFw-73D-19@gated-at.bofh.it>
In reply to#1595006
On Wed, 8 Mar 2017, Josh Poimboeuf wrote:

> On Wed, Mar 08, 2017 at 10:16:00AM +0100, Jiri Kosina wrote:
> > From: Jiri Kosina <jkosina@suse.cz>
> > 
> > klp_mutex is shared between core.c and transition.c, and as such would 
> > rather be properly located in livepatch.h so that we don't have to play 
> > 'extern' games from .c sources.
> > 
> > This also silences sparse warning (wrongly) suggesting that klp_mutex 
> > should be defined static.
> 
> include/linux/livepatch.h is the public interface, whereas klp_mutex is
> a private implementation detail.  I think it would be a better fit in
> kernel/livepatch/core.h (though that file doesn't yet exist).

Yes, that would be ideal and I thought about it too. linux/livepatch.h is 
suboptimal fallback, because I did not find it necessary to have another 
header file just because of klp_mutex. But I'll be all for it, if everyone 
is in favour.

Miroslav

[toc] | [prev] | [next] | [standalone]

#1595173 — [PATCH v2] livepatch: make klp_mutex proper part of API

FromJiri Kosina <jikos@kernel.org>
Date2017-03-08 14:30 +0100
Subject[PATCH v2] livepatch: make klp_mutex proper part of API
Message-ID<tiJPc-77d-27@gated-at.bofh.it>
In reply to#1595006
From: Jiri Kosina <jkosina@suse.cz>

klp_mutex is shared between core.c and transition.c, and as such would 
rather be properly located in a header so that we don't have to play 
'extern' games from .c sources.

This also silences sparse warning (wrongly) suggesting that klp_mutex 
should be defined static.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 kernel/livepatch/core.c       | 1 +
 kernel/livepatch/core.h       | 6 ++++++
 kernel/livepatch/transition.c | 3 +--
 3 files changed, 8 insertions(+), 2 deletions(-)
 create mode 100644 kernel/livepatch/core.h

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 6844c1213df8..47402b8b3990 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -31,6 +31,7 @@
 #include <linux/moduleloader.h>
 #include <linux/completion.h>
 #include <asm/cacheflush.h>
+#include "core.h"
 #include "patch.h"
 #include "transition.h"
 
diff --git a/kernel/livepatch/core.h b/kernel/livepatch/core.h
new file mode 100644
index 000000000000..c74f24c47837
--- /dev/null
+++ b/kernel/livepatch/core.h
@@ -0,0 +1,6 @@
+#ifndef _LIVEPATCH_CORE_H
+#define _LIVEPATCH_CORE_H
+
+extern struct mutex klp_mutex;
+
+#endif /* _LIVEPATCH_CORE_H */
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 0ab7abd53b0b..2de09e0c4e5c 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -21,6 +21,7 @@
 
 #include <linux/cpu.h>
 #include <linux/stacktrace.h>
+#include "core.h"
 #include "patch.h"
 #include "transition.h"
 #include "../sched/sched.h"
@@ -28,8 +29,6 @@
 #define MAX_STACK_ENTRIES  100
 #define STACK_ERR_BUF_SIZE 128
 
-extern struct mutex klp_mutex;
-
 struct klp_patch *klp_transition_patch;
 
 static int klp_target_state = KLP_UNDEFINED;

-- 
Jiri Kosina
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1595178 — Re: [PATCH v2] livepatch: make klp_mutex proper part of API

FromMiroslav Benes <mbenes@suse.cz>
Date2017-03-08 14:40 +0100
SubjectRe: [PATCH v2] livepatch: make klp_mutex proper part of API
Message-ID<tiJYR-7cO-3@gated-at.bofh.it>
In reply to#1595173
On Wed, 8 Mar 2017, Jiri Kosina wrote:

> From: Jiri Kosina <jkosina@suse.cz>
> 
> klp_mutex is shared between core.c and transition.c, and as such would 
> rather be properly located in a header so that we don't have to play 
> 'extern' games from .c sources.
> 
> This also silences sparse warning (wrongly) suggesting that klp_mutex 
> should be defined static.
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>

Acked-by: Miroslav Benes <mbenes@suse.cz>

Miroslav

[toc] | [prev] | [next] | [standalone]

#1595181 — Re: [PATCH v2] livepatch: make klp_mutex proper part of API

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-08 14:40 +0100
SubjectRe: [PATCH v2] livepatch: make klp_mutex proper part of API
Message-ID<tiJYR-7cO-17@gated-at.bofh.it>
In reply to#1595173
On Wed, Mar 08, 2017 at 02:27:05PM +0100, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
> 
> klp_mutex is shared between core.c and transition.c, and as such would 
> rather be properly located in a header so that we don't have to play 
> 'extern' games from .c sources.
> 
> This also silences sparse warning (wrongly) suggesting that klp_mutex 
> should be defined static.
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1595402

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-03-08 18:30 +0100
Message-ID<tiJFw-73D-21@gated-at.bofh.it>
In reply to#1595006
On Wed, Mar 08, 2017 at 10:16:00AM +0100, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
> 
> klp_mutex is shared between core.c and transition.c, and as such would 
> rather be properly located in livepatch.h so that we don't have to play 
> 'extern' games from .c sources.
> 
> This also silences sparse warning (wrongly) suggesting that klp_mutex 
> should be defined static.

include/linux/livepatch.h is the public interface, whereas klp_mutex is
a private implementation detail.  I think it would be a better fit in
kernel/livepatch/core.h (though that file doesn't yet exist).

-- 
Josh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web