forked from mirrors/linux
		
	hw-breakpoints: Use overflow handler instead of the event callback
struct perf_event::event callback was called when a breakpoint triggers. But this is a rather opaque callback, pretty tied-only to the breakpoint API and not really integrated into perf as it triggers even when we don't overflow. We prefer to use overflow_handler() as it fits into the perf events rules, being called only when we overflow. Reported-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com> Cc: Paul Mackerras <paulus@samba.org> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: "K. Prasad" <prasad@linux.vnet.ibm.com>
This commit is contained in:
		
							parent
							
								
									2f0993e0fb
								
							
						
					
					
						commit
						b326e9560a
					
				
					 8 changed files with 48 additions and 57 deletions
				
			
		|  | @ -362,8 +362,7 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp, | |||
| 		return ret; | ||||
| 	} | ||||
| 
 | ||||
| 	if (bp->callback) | ||||
| 		ret = arch_store_info(bp); | ||||
| 	ret = arch_store_info(bp); | ||||
| 
 | ||||
| 	if (ret < 0) | ||||
| 		return ret; | ||||
|  | @ -519,7 +518,7 @@ static int __kprobes hw_breakpoint_handler(struct die_args *args) | |||
| 			break; | ||||
| 		} | ||||
| 
 | ||||
| 		(bp->callback)(bp, args->regs); | ||||
| 		perf_bp_event(bp, args->regs); | ||||
| 
 | ||||
| 		rcu_read_unlock(); | ||||
| 	} | ||||
|  |  | |||
|  | @ -555,7 +555,9 @@ static int genregs_set(struct task_struct *target, | |||
| 	return ret; | ||||
| } | ||||
| 
 | ||||
| static void ptrace_triggered(struct perf_event *bp, void *data) | ||||
| static void ptrace_triggered(struct perf_event *bp, int nmi, | ||||
| 			     struct perf_sample_data *data, | ||||
| 			     struct pt_regs *regs) | ||||
| { | ||||
| 	int i; | ||||
| 	struct thread_struct *thread = &(current->thread); | ||||
|  | @ -599,7 +601,7 @@ ptrace_modify_breakpoint(struct perf_event *bp, int len, int type, | |||
| { | ||||
| 	int err; | ||||
| 	int gen_len, gen_type; | ||||
| 	DEFINE_BREAKPOINT_ATTR(attr); | ||||
| 	struct perf_event_attr attr; | ||||
| 
 | ||||
| 	/*
 | ||||
| 	 * We shoud have at least an inactive breakpoint at this | ||||
|  | @ -721,9 +723,10 @@ static int ptrace_set_breakpoint_addr(struct task_struct *tsk, int nr, | |||
| { | ||||
| 	struct perf_event *bp; | ||||
| 	struct thread_struct *t = &tsk->thread; | ||||
| 	DEFINE_BREAKPOINT_ATTR(attr); | ||||
| 	struct perf_event_attr attr; | ||||
| 
 | ||||
| 	if (!t->ptrace_bps[nr]) { | ||||
| 		hw_breakpoint_init(&attr); | ||||
| 		/*
 | ||||
| 		 * Put stub len and type to register (reserve) an inactive but | ||||
| 		 * correct bp | ||||
|  |  | |||
|  | @ -20,19 +20,16 @@ enum { | |||
| 
 | ||||
| #ifdef CONFIG_HAVE_HW_BREAKPOINT | ||||
| 
 | ||||
| /* As it's for in-kernel or ptrace use, we want it to be pinned */ | ||||
| #define DEFINE_BREAKPOINT_ATTR(name)	\ | ||||
| struct perf_event_attr name = {		\ | ||||
| 	.type = PERF_TYPE_BREAKPOINT,	\ | ||||
| 	.size = sizeof(name),		\ | ||||
| 	.pinned = 1,			\ | ||||
| }; | ||||
| 
 | ||||
| static inline void hw_breakpoint_init(struct perf_event_attr *attr) | ||||
| { | ||||
| 	attr->type = PERF_TYPE_BREAKPOINT; | ||||
| 	attr->size = sizeof(*attr); | ||||
| 	/*
 | ||||
| 	 * As it's for in-kernel or ptrace use, we want it to be pinned | ||||
| 	 * and to call its callback every hits. | ||||
| 	 */ | ||||
| 	attr->pinned = 1; | ||||
| 	attr->sample_period = 1; | ||||
| } | ||||
| 
 | ||||
| static inline unsigned long hw_breakpoint_addr(struct perf_event *bp) | ||||
|  | @ -52,7 +49,7 @@ static inline int hw_breakpoint_len(struct perf_event *bp) | |||
| 
 | ||||
| extern struct perf_event * | ||||
| register_user_hw_breakpoint(struct perf_event_attr *attr, | ||||
| 			    perf_callback_t triggered, | ||||
| 			    perf_overflow_handler_t triggered, | ||||
| 			    struct task_struct *tsk); | ||||
| 
 | ||||
| /* FIXME: only change from the attr, and don't unregister */ | ||||
|  | @ -64,12 +61,12 @@ modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr); | |||
|  */ | ||||
| extern struct perf_event * | ||||
| register_wide_hw_breakpoint_cpu(struct perf_event_attr *attr, | ||||
| 				perf_callback_t triggered, | ||||
| 				perf_overflow_handler_t	triggered, | ||||
| 				int cpu); | ||||
| 
 | ||||
| extern struct perf_event ** | ||||
| register_wide_hw_breakpoint(struct perf_event_attr *attr, | ||||
| 			    perf_callback_t triggered); | ||||
| 			    perf_overflow_handler_t triggered); | ||||
| 
 | ||||
| extern int register_perf_hw_breakpoint(struct perf_event *bp); | ||||
| extern int __register_perf_hw_breakpoint(struct perf_event *bp); | ||||
|  | @ -90,18 +87,18 @@ static inline struct arch_hw_breakpoint *counter_arch_bp(struct perf_event *bp) | |||
| 
 | ||||
| static inline struct perf_event * | ||||
| register_user_hw_breakpoint(struct perf_event_attr *attr, | ||||
| 			    perf_callback_t triggered, | ||||
| 			    perf_overflow_handler_t triggered, | ||||
| 			    struct task_struct *tsk)	{ return NULL; } | ||||
| static inline struct perf_event * | ||||
| modify_user_hw_breakpoint(struct perf_event *bp, | ||||
| 			  struct perf_event_attr *attr)	{ return NULL; } | ||||
| static inline struct perf_event * | ||||
| register_wide_hw_breakpoint_cpu(struct perf_event_attr *attr, | ||||
| 				perf_callback_t triggered, | ||||
| 				perf_overflow_handler_t	 triggered, | ||||
| 				int cpu)		{ return NULL; } | ||||
| static inline struct perf_event ** | ||||
| register_wide_hw_breakpoint(struct perf_event_attr *attr, | ||||
| 			    perf_callback_t triggered)	{ return NULL; } | ||||
| 			    perf_overflow_handler_t triggered)	{ return NULL; } | ||||
| static inline int | ||||
| register_perf_hw_breakpoint(struct perf_event *bp)	{ return -ENOSYS; } | ||||
| static inline int | ||||
|  |  | |||
|  | @ -565,10 +565,13 @@ struct perf_pending_entry { | |||
| 	void (*func)(struct perf_pending_entry *); | ||||
| }; | ||||
| 
 | ||||
| typedef void (*perf_callback_t)(struct perf_event *, void *); | ||||
| 
 | ||||
| struct perf_sample_data; | ||||
| 
 | ||||
| typedef void (*perf_callback_t)(struct perf_event *, void *); | ||||
| typedef void (*perf_overflow_handler_t)(struct perf_event *, int, | ||||
| 					struct perf_sample_data *, | ||||
| 					struct pt_regs *regs); | ||||
| 
 | ||||
| /**
 | ||||
|  * struct perf_event - performance event kernel representation: | ||||
|  */ | ||||
|  | @ -660,9 +663,7 @@ struct perf_event { | |||
| 	struct pid_namespace		*ns; | ||||
| 	u64				id; | ||||
| 
 | ||||
| 	void (*overflow_handler)(struct perf_event *event, | ||||
| 			int nmi, struct perf_sample_data *data, | ||||
| 			struct pt_regs *regs); | ||||
| 	perf_overflow_handler_t		overflow_handler; | ||||
| 
 | ||||
| #ifdef CONFIG_EVENT_PROFILE | ||||
| 	struct event_filter		*filter; | ||||
|  | @ -779,7 +780,7 @@ extern struct perf_event * | |||
| perf_event_create_kernel_counter(struct perf_event_attr *attr, | ||||
| 				int cpu, | ||||
| 				pid_t pid, | ||||
| 				perf_callback_t callback); | ||||
| 				perf_overflow_handler_t callback); | ||||
| extern u64 perf_event_read_value(struct perf_event *event, | ||||
| 				 u64 *enabled, u64 *running); | ||||
| 
 | ||||
|  |  | |||
|  | @ -259,7 +259,7 @@ void release_bp_slot(struct perf_event *bp) | |||
| } | ||||
| 
 | ||||
| 
 | ||||
| int __register_perf_hw_breakpoint(struct perf_event *bp) | ||||
| int register_perf_hw_breakpoint(struct perf_event *bp) | ||||
| { | ||||
| 	int ret; | ||||
| 
 | ||||
|  | @ -276,19 +276,12 @@ int __register_perf_hw_breakpoint(struct perf_event *bp) | |||
| 	 * This is a quick hack that will be removed soon, once we remove | ||||
| 	 * the tmp breakpoints from ptrace | ||||
| 	 */ | ||||
| 	if (!bp->attr.disabled || bp->callback == perf_bp_event) | ||||
| 	if (!bp->attr.disabled || !bp->overflow_handler) | ||||
| 		ret = arch_validate_hwbkpt_settings(bp, bp->ctx->task); | ||||
| 
 | ||||
| 	return ret; | ||||
| } | ||||
| 
 | ||||
| int register_perf_hw_breakpoint(struct perf_event *bp) | ||||
| { | ||||
| 	bp->callback = perf_bp_event; | ||||
| 
 | ||||
| 	return __register_perf_hw_breakpoint(bp); | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  * register_user_hw_breakpoint - register a hardware breakpoint for user space | ||||
|  * @attr: breakpoint attributes | ||||
|  | @ -297,7 +290,7 @@ int register_perf_hw_breakpoint(struct perf_event *bp) | |||
|  */ | ||||
| struct perf_event * | ||||
| register_user_hw_breakpoint(struct perf_event_attr *attr, | ||||
| 			    perf_callback_t triggered, | ||||
| 			    perf_overflow_handler_t triggered, | ||||
| 			    struct task_struct *tsk) | ||||
| { | ||||
| 	return perf_event_create_kernel_counter(attr, -1, tsk->pid, triggered); | ||||
|  | @ -322,7 +315,7 @@ modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr) | |||
| 	unregister_hw_breakpoint(bp); | ||||
| 
 | ||||
| 	return perf_event_create_kernel_counter(attr, -1, bp->ctx->task->pid, | ||||
| 						bp->callback); | ||||
| 						bp->overflow_handler); | ||||
| } | ||||
| EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint); | ||||
| 
 | ||||
|  | @ -347,7 +340,7 @@ EXPORT_SYMBOL_GPL(unregister_hw_breakpoint); | |||
|  */ | ||||
| struct perf_event ** | ||||
| register_wide_hw_breakpoint(struct perf_event_attr *attr, | ||||
| 			    perf_callback_t triggered) | ||||
| 			    perf_overflow_handler_t triggered) | ||||
| { | ||||
| 	struct perf_event **cpu_events, **pevent, *bp; | ||||
| 	long err; | ||||
|  |  | |||
|  | @ -4286,15 +4286,8 @@ static void bp_perf_event_destroy(struct perf_event *event) | |||
| static const struct pmu *bp_perf_event_init(struct perf_event *bp) | ||||
| { | ||||
| 	int err; | ||||
| 	/*
 | ||||
| 	 * The breakpoint is already filled if we haven't created the counter | ||||
| 	 * through perf syscall | ||||
| 	 * FIXME: manage to get trigerred to NULL if it comes from syscalls | ||||
| 	 */ | ||||
| 	if (!bp->callback) | ||||
| 		err = register_perf_hw_breakpoint(bp); | ||||
| 	else | ||||
| 		err = __register_perf_hw_breakpoint(bp); | ||||
| 
 | ||||
| 	err = register_perf_hw_breakpoint(bp); | ||||
| 	if (err) | ||||
| 		return ERR_PTR(err); | ||||
| 
 | ||||
|  | @ -4390,7 +4383,7 @@ perf_event_alloc(struct perf_event_attr *attr, | |||
| 		   struct perf_event_context *ctx, | ||||
| 		   struct perf_event *group_leader, | ||||
| 		   struct perf_event *parent_event, | ||||
| 		   perf_callback_t callback, | ||||
| 		   perf_overflow_handler_t overflow_handler, | ||||
| 		   gfp_t gfpflags) | ||||
| { | ||||
| 	const struct pmu *pmu; | ||||
|  | @ -4433,10 +4426,10 @@ perf_event_alloc(struct perf_event_attr *attr, | |||
| 
 | ||||
| 	event->state		= PERF_EVENT_STATE_INACTIVE; | ||||
| 
 | ||||
| 	if (!callback && parent_event) | ||||
| 		callback = parent_event->callback; | ||||
| 	if (!overflow_handler && parent_event) | ||||
| 		overflow_handler = parent_event->overflow_handler; | ||||
| 	 | ||||
| 	event->callback	= callback; | ||||
| 	event->overflow_handler	= overflow_handler; | ||||
| 
 | ||||
| 	if (attr->disabled) | ||||
| 		event->state = PERF_EVENT_STATE_OFF; | ||||
|  | @ -4776,7 +4769,8 @@ SYSCALL_DEFINE5(perf_event_open, | |||
|  */ | ||||
| struct perf_event * | ||||
| perf_event_create_kernel_counter(struct perf_event_attr *attr, int cpu, | ||||
| 				 pid_t pid, perf_callback_t callback) | ||||
| 				 pid_t pid, | ||||
| 				 perf_overflow_handler_t overflow_handler) | ||||
| { | ||||
| 	struct perf_event *event; | ||||
| 	struct perf_event_context *ctx; | ||||
|  | @ -4793,7 +4787,7 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr, int cpu, | |||
| 	} | ||||
| 
 | ||||
| 	event = perf_event_alloc(attr, cpu, ctx, NULL, | ||||
| 				     NULL, callback, GFP_KERNEL); | ||||
| 				 NULL, overflow_handler, GFP_KERNEL); | ||||
| 	if (IS_ERR(event)) { | ||||
| 		err = PTR_ERR(event); | ||||
| 		goto err_put_context; | ||||
|  |  | |||
|  | @ -79,11 +79,12 @@ void ksym_collect_stats(unsigned long hbp_hit_addr) | |||
| } | ||||
| #endif /* CONFIG_PROFILE_KSYM_TRACER */ | ||||
| 
 | ||||
| void ksym_hbp_handler(struct perf_event *hbp, void *data) | ||||
| void ksym_hbp_handler(struct perf_event *hbp, int nmi, | ||||
| 		      struct perf_sample_data *data, | ||||
| 		      struct pt_regs *regs) | ||||
| { | ||||
| 	struct ring_buffer_event *event; | ||||
| 	struct ksym_trace_entry *entry; | ||||
| 	struct pt_regs *regs = data; | ||||
| 	struct ring_buffer *buffer; | ||||
| 	int pc; | ||||
| 
 | ||||
|  |  | |||
|  | @ -41,7 +41,9 @@ module_param_string(ksym, ksym_name, KSYM_NAME_LEN, S_IRUGO); | |||
| MODULE_PARM_DESC(ksym, "Kernel symbol to monitor; this module will report any" | ||||
| 			" write operations on the kernel symbol"); | ||||
| 
 | ||||
| static void sample_hbp_handler(struct perf_event *temp, void *data) | ||||
| static void sample_hbp_handler(struct perf_event *bp, int nmi, | ||||
| 			       struct perf_sample_data *data, | ||||
| 			       struct pt_regs *regs) | ||||
| { | ||||
| 	printk(KERN_INFO "%s value is changed\n", ksym_name); | ||||
| 	dump_stack(); | ||||
|  | @ -51,8 +53,9 @@ static void sample_hbp_handler(struct perf_event *temp, void *data) | |||
| static int __init hw_break_module_init(void) | ||||
| { | ||||
| 	int ret; | ||||
| 	DEFINE_BREAKPOINT_ATTR(attr); | ||||
| 	struct perf_event_attr attr; | ||||
| 
 | ||||
| 	hw_breakpoint_init(&attr); | ||||
| 	attr.bp_addr = kallsyms_lookup_name(ksym_name); | ||||
| 	attr.bp_len = HW_BREAKPOINT_LEN_4; | ||||
| 	attr.bp_type = HW_BREAKPOINT_W | HW_BREAKPOINT_R; | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Frederic Weisbecker
						Frederic Weisbecker