Re: Q: perf_event && task->ptrace_bps[]

[Oleg Nesterov <oleg@redhat.com>]

On 01/19, Frederic Weisbecker wrote:
>
> On Wed, Jan 19, 2011 at 04:37:46PM +0100, Oleg Nesterov wrote:
> >
> > I think we can reuse perf_event_mutex for this. Not very good too,
> > but simple. But this depends on what can we do under this mutex...
>
> That could work. I feel a bit uncomfortable to use a perf related
> mutex for that though. I can't figure out any deadlock with the current
> state, but if we are going to use that solution, perf events will be
> created/destroyed/disabled/enabled under that mutex.

No, I didn't mean create/destroy under that mutex, but

> Dunno, that doesn't seem to be a good use of perf_event_mutex.

I agree anyway.

> OTOH I can drop
> more of them for the no-running-breakpoint case from thread_struct
> in a subsequent task.

Hmm. Can't understand what do you mean. Just curious, could you explain?

> Note the problem touches more archs than x86. Basically every
> arch that use breakpoint use a similar scheme that must be fixed.

Yes. Perhaps we should try to unify some code... Say, can't we move
->ptrace_bps[] to task_struct?


> +void ptrace_put_breakpoints(struct task_struct *tsk)
> +{
> +	if (!atomic_dec_return(&tsk->ptrace_bp_refcnt))
> +		flush_ptrace_hw_breakpoint(tsk);

(minor nit, atomic_dec_and_test() looks more natural)


I think the patch is correct and should fix the problem.

Thanks!

Oleg.