[PATCH] livepatch: make klp_mutex proper part of API

[PATCH] livepatch: make klp_mutex proper part of API

[Jiri Kosina]

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

Re: [PATCH] livepatch: make klp_mutex proper part of API

[Miroslav Benes]

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

Re: [PATCH] livepatch: make klp_mutex proper part of API

[Josh Poimboeuf]

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

Re: [PATCH] livepatch: make klp_mutex proper part of API

[Miroslav Benes]

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

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

[Jiri Kosina]

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

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

[Miroslav Benes]

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

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

[Josh Poimboeuf]

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