Re: [PATCH v2 3/7] x86, ptrace: Ifdef HW_BREAKPOINTS code in ptrace

[Josh T. <jo...@jo...>]

On Mon, Mar 10, 2014 at 03:42:55PM +0100, Oleg Nesterov wrote:
> On 03/10, Oleg Nesterov wrote:
> >
> > On 03/09, Josh Triplett wrote:
> > >
> > > +static unsigned long ptrace_get_debugreg(struct task_struct *tsk, int n)
> > > +{
> > > +	struct thread_struct *thread = &tsk->thread;
> > > +
> > > +	if (n == 6)
> > > +		return thread->debugreg6;
> > > +	return 0;
> > > +}
> > > +
> > > +/* Without breakpoints only handle DR6 */
> > > +static int ptrace_set_debugreg(struct task_struct *tsk, int n,
> > > +			       unsigned long val)
> > > +{
> > > +	struct thread_struct *thread = &tsk->thread;
> > > +	int rc = -EIO;
> > > +
> > > +	if (n == 6) {
> > > +		thread->debugreg6 = val;
> > > +		rc = 0;
> > > +	}
> > > +	return rc;
> > > +}
> > > +
> > > +#endif
> >
> > OK, but then perhaps this patch should also make thread_struct->ptrace_bps[]
> > depend on CONFIG_HW_BREAKPOINTS ?
> 
> and ->ptrace_dr7 at least.

Those do seem like nice optimizations to reduce the size of
thread_struct.

> And in fact, why do we need ptrace_get/set_debugreg(DR6) without HW_BREAKPOINTS?

Good question.  Andi?

> Perhaps get/set should just return the error?

Perhaps.

> > But I won't insist if you prefer to do this later.
> 
> Yes.

The optimization of thread_struct I would prefer to defer until after
this patch series, yes.

But if there's a better way to write ptrace_{g,s}et_debugreg when
!CONFIG_HW_BREAKPOINTS, I'll happily change that here.

- Josh Triplett