Re: Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

From: Heiko Carstens
Date: Fri Aug 23 2013 - 04:10:36 EST


On Fri, Aug 23, 2013 at 01:31:23PM +0900, Masami Hiramatsu wrote:
> (2013/08/22 14:52), Heiko Carstens wrote:
> > Therefore I need to different insn slot caches, where the slots are either
> > allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
> > or module_alloc(PAGE_SIZE) for modules.
> >
> > I can't have a single cache which satifies both areas.
>
> Oh, I see.
> Indeed, that enough reason to add a new cache... By the way, is there
> any way to implement it without new kconfig like DMAPROBE and dma flag?
> AFAICS, since such flag is strongly depends on the s390 arch, I don't
> like to put it in kernel/kprobes.c.
>
> Perhaps, we can make insn slot more generic, e.g. create new slot type
> with passing page allocator.

Something like below?
(only compile tested and on top of the previous patches).

I'm not sure, since that would expose struct kprobe_insn_cache.

arch/Kconfig | 7 -------
arch/s390/Kconfig | 1 -
arch/s390/kernel/kprobes.c | 20 ++++++++++++++++++
include/linux/kprobes.h | 14 ++++++++-----
kernel/kprobes.c | 51 ++++++++++++++++------------------------------
5 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 7010d68..1feb169 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -76,13 +76,6 @@ config OPTPROBES
depends on KPROBES && HAVE_OPTPROBES
depends on !PREEMPT

-config DMAPROBES
- bool
- help
- Architectures may want to put kprobes instruction slots into
- the dma memory region. E.g. s390 has the kernel image in the
- dma memory region but the module area far away.
-
config KPROBES_ON_FTRACE
def_bool y
depends on KPROBES && HAVE_KPROBES_ON_FTRACE
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index ce389a9..8a4cae7 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -96,7 +96,6 @@ config S390
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_EXTABLE_SORT
select CLONE_BACKWARDS2
- select DMAPROBES if KPROBES
select GENERIC_CLOCKEVENTS
select GENERIC_CPU_DEVICES if !SMP
select GENERIC_SMP_IDLE_THREAD
diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
index bc1071c..cb7ac9e 100644
--- a/arch/s390/kernel/kprobes.c
+++ b/arch/s390/kernel/kprobes.c
@@ -37,6 +37,26 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);

struct kretprobe_blackpoint kretprobe_blacklist[] = { };

+DEFINE_INSN_CACHE_OPS(dmainsn);
+
+static void *alloc_dmainsn_page(void)
+{
+ return (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
+}
+
+static void free_dmainsn_page(void *page)
+{
+ free_page((unsigned long)page);
+}
+
+struct kprobe_insn_cache kprobe_dmainsn_slots = {
+ .mutex = __MUTEX_INITIALIZER(kprobe_dmainsn_slots.mutex),
+ .alloc = alloc_dmainsn_page,
+ .free = free_dmainsn_page,
+ .pages = LIST_HEAD_INIT(kprobe_dmainsn_slots.pages),
+ .insn_size = MAX_INSN_SIZE,
+};
+
static int __kprobes is_prohibited_opcode(kprobe_opcode_t *insn)
{
switch (insn[0] >> 8) {
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index a5290f6..4e96827 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -266,7 +266,15 @@ extern int arch_init_kprobes(void);
extern void show_registers(struct pt_regs *regs);
extern void kprobes_inc_nmissed_count(struct kprobe *p);

-struct kprobe_insn_cache;
+struct kprobe_insn_cache {
+ struct mutex mutex;
+ void *(*alloc)(void); /* allocate insn page */
+ void (*free)(void *); /* free insn page */
+ struct list_head pages; /* list of kprobe_insn_page */
+ size_t insn_size; /* size of instruction slot */
+ int nr_garbage;
+};
+
extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
extern void __free_insn_slot(struct kprobe_insn_cache *c,
kprobe_opcode_t *slot, int dirty);
@@ -321,10 +329,6 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table,

#endif /* CONFIG_OPTPROBES */

-#ifdef CONFIG_DMAPROBES
-DEFINE_INSN_CACHE_OPS(dmainsn);
-#endif
-
#ifdef CONFIG_KPROBES_ON_FTRACE
extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *ops, struct pt_regs *regs);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 3b8b073..a0d367a 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -112,9 +112,9 @@ static struct kprobe_blackpoint kprobe_blacklist[] = {
struct kprobe_insn_page {
struct list_head list;
kprobe_opcode_t *insns; /* Page of instruction slots */
+ struct kprobe_insn_cache *cache;
int nused;
int ngarbage;
- bool dma_alloc;
char slot_used[];
};

@@ -122,14 +122,6 @@ struct kprobe_insn_page {
(offsetof(struct kprobe_insn_page, slot_used) + \
(sizeof(char) * (slots)))

-struct kprobe_insn_cache {
- struct mutex mutex;
- struct list_head pages; /* list of kprobe_insn_page */
- size_t insn_size; /* size of instruction slot */
- int nr_garbage;
- bool dma_alloc;
-};
-
static int slots_per_page(struct kprobe_insn_cache *c)
{
return PAGE_SIZE/(c->insn_size * sizeof(kprobe_opcode_t));
@@ -141,12 +133,23 @@ enum kprobe_slot_state {
SLOT_USED = 2,
};

+static void *alloc_insn_page(void)
+{
+ return module_alloc(PAGE_SIZE);
+}
+
+static void free_insn_page(void *page)
+{
+ module_free(NULL, page);
+}
+
struct kprobe_insn_cache kprobe_insn_slots = {
.mutex = __MUTEX_INITIALIZER(kprobe_insn_slots.mutex),
+ .alloc = alloc_insn_page,
+ .free = free_insn_page,
.pages = LIST_HEAD_INIT(kprobe_insn_slots.pages),
.insn_size = MAX_INSN_SIZE,
.nr_garbage = 0,
- .dma_alloc = false,
};
static int __kprobes collect_garbage_slots(struct kprobe_insn_cache *c);

@@ -192,10 +195,7 @@ kprobe_opcode_t __kprobes *__get_insn_slot(struct kprobe_insn_cache *c)
* kernel image and loaded module images reside. This is required
* so x86_64 can correctly handle the %rip-relative fixups.
*/
- if (c->dma_alloc)
- kip->insns = (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
- else
- kip->insns = module_alloc(PAGE_SIZE);
+ kip->insns = c->alloc();
if (!kip->insns) {
kfree(kip);
goto out;
@@ -205,7 +205,7 @@ kprobe_opcode_t __kprobes *__get_insn_slot(struct kprobe_insn_cache *c)
kip->slot_used[0] = SLOT_USED;
kip->nused = 1;
kip->ngarbage = 0;
- kip->dma_alloc = c->dma_alloc;
+ kip->cache = c;
list_add(&kip->list, &c->pages);
slot = kip->insns;
out:
@@ -227,10 +227,7 @@ static int __kprobes collect_one_slot(struct kprobe_insn_page *kip, int idx)
*/
if (!list_is_singular(&kip->list)) {
list_del(&kip->list);
- if (kip->dma_alloc)
- free_page((unsigned long)kip->insns);
- else
- module_free(NULL, kip->insns);
+ kip->cache->free(kip->insns);
kfree(kip);
}
return 1;
@@ -291,23 +288,11 @@ out:
/* For optimized_kprobe buffer */
struct kprobe_insn_cache kprobe_optinsn_slots = {
.mutex = __MUTEX_INITIALIZER(kprobe_optinsn_slots.mutex),
+ .alloc = alloc_insn_page,
+ .free = free_insn_page,
.pages = LIST_HEAD_INIT(kprobe_optinsn_slots.pages),
/* .insn_size is initialized later */
.nr_garbage = 0,
- .dma_alloc = false,
-};
-#endif
-#ifdef CONFIG_DMAPROBES
-/*
- * Special buffer for architectures which require insn slots
- * to be in the GFP_DMA memory range.
- */
-struct kprobe_insn_cache kprobe_dmainsn_slots = {
- .mutex = __MUTEX_INITIALIZER(kprobe_dmainsn_slots.mutex),
- .pages = LIST_HEAD_INIT(kprobe_dmainsn_slots.pages),
- .insn_size = MAX_INSN_SIZE,
- .nr_garbage = 0,
- .dma_alloc = true,
};
#endif
#endif
--
1.8.2.3

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