On Mon, Feb 22, 2016 at 01:34:05AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote: > >From: Luis R. Rodriguez [mailto:mcgrof@xxxxxxxxxx] > > > >kprobe makes use of two custom sections: > > > >type name begin end > >init.data _kprobe_blacklist __start_kprobe_blacklist __stop_kprobe_blacklist > >text .kprobes.text __kprobes_text_start __kprobes_text_end > > > >Port these to the linker table generic solution. This lets > >us remove all the custom kprobe section declarations on the > >linker script. > > > >Tested with CONFIG_KPROBES_SANITY_TEST, it passes with: > > > >Kprobe smoke test: started > >Kprobe smoke test: passed successfully > > > >Then tested CONFIG_SAMPLE_KPROBES on do_fork, and the > >kprobe bites and kicks as expected. Lastly tried registering > >a kprobe on a kprobe blacklisted symbol (NOKPROBE_SYMBOL()), > >and confirms that fails to work. > > Could you also check to run the testcases by using ftracetest as below? > > $ cd tools/testing/selftests/ftrace/ > $ sudo ./ftracetest Sure, it all passed: $ sudo ./ftracetest === Ftrace unit tests === [1] Basic trace file check [PASS] [2] Basic test for tracers [PASS] [3] Basic trace clock test [PASS] [4] Basic event tracing check [PASS] [5] event tracing - enable/disable with event level files [PASS] [6] event tracing - enable/disable with subsystem level files [PASS] [7] event tracing - enable/disable with top level files [PASS] [8] ftrace - function graph filters with stack tracer [PASS] [9] ftrace - function graph filters [PASS] [10] ftrace - function profiler with function tracing [PASS] [11] Test creation and deletion of trace instances [PASS] [12] Kprobe dynamic event - adding and removing [PASS] [13] Kprobe dynamic event - busy event check [PASS] [14] Kprobe dynamic event with arguments [PASS] [15] Kprobe dynamic event with function tracer [PASS] [16] Kretprobe dynamic event with arguments [PASS] # of passed: 16 # of failed: 0 # of unresolved: 0 # of untested: 0 # of unsupported: 0 # of xfailed: 0 # of undefined(test bug): 0 > And I'm not sure about linker table. So there's really a few parts to the linker table work, out of the ones that relate to kprobes: * linker tables try to generalize our section use, and provide some helpers for common section use * provides helpers to make it easier to make custom section, but by re-using standard sections * when a custom section uses standard sections and helpers we also get a few gains: - developers reviewing code can more easily get a quicker understanding and have expectations set of how the code feature using the custom section could be used - people grep'ing on the kernel can more easily find specific types of custom section use by looking for the type of interest - developers adding features do not need to modify any linker scripts (vmlinux.lds.S) to make use of custom sections In kprobe's case, since it uses two custom sections, we have only a small use for the first case: .kprobe.text is just used as a place holder for future developer annotated special cased executable code. It also makes use of the generic helpers: LINKTABLE_ADDR_WITHIN(), LINKTABLE_START(), LINKTABLE_END(). The second use case, for the _kprobe_blacklist, makes much more use of the more advanced linker table helpers, for instance the iterator LINKTABLE_FOR_EACH(). For both though we now have each custom section's actual section clearly highlighted: * kprobes: .text (SECTION_TEXT) * kprobe blacklist: init.data (SECTION_INIT_DATA) A reader / developer can more easily gain an understanding of how the above custom sections could be used just by its type. Another feature of linker tables, but outside of the scope of how kprobe would use linker tables, is the ability to enable folks to avoid code bit rot by using table-$(CONFIG_FOO) instead of oby-$(CONFIG_FOO) on init paths of code but since this is outside of the scope of how kprobe would use I leave that just as a reference as another part of linker table. Unless of course you want to make people force compile all kprobe support code but only have it linked in when actually enabled. That would be outside of the scope and purpose of this patch though. > Is that possible to support > __kprobes prefix, which moves the functions into kprobes.text? Absolutely, the respective change was just to annotate and make it clear the section kprobes were using: -# define __kprobes __attribute__((__section__(".kprobes.text"))) +#include <linux/sections.h> +# define __kprobes __attribute__((__section__(SECTION_TBL(SECTION_TEXT, kprobes, all)))) > Actually, I'm on the way to replacing __kprobes to NOKPROBE_SYMBOL > macro, since NOKPROBE_SYMBOL() doesn't effect the kernel text itself. > On x86, it is already replaced (see commit 820aede0209a), and same > work should be done on other archs. So, could you hold this after > that? Sure. > I think we should remove .kprobes.text first You mean just remove the incorrect users of .kprobes.text because as I read what you described above we have abuse of __kprobes use to protect against kprobes being introduced on those routines, and we should be using NOKPROBE_SYMBOL() instead. So from what I gather its not that you will remove .kprobes.text but rather clean our current abuse of __kprobes for protection to use NOKPROBE_SYMBOL() instead. Is that right? > and move to linker table. Sure, can you Cc me on your patches? I can follow up later. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
