Re: [PATCH 04/16] perf: Add ability to attach user stack dump tosample

[Frederic Weisbecker]

On Thu, May 24, 2012 at 12:51:20PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-05-23 at 21:32 +0200, Jiri Olsa wrote:
> > +static void
> > +perf_output_sample_ustack(struct perf_output_handle *handle, u64 dump_size,
> > +                         struct pt_regs *regs)
> > +{
> > +       u64 size;
> > +
> > +       /* Case of a kernel thread, nothing to dump */
> > +       if (!regs) {
> > +               size = 0;
> > +               perf_output_put(handle, size);
> > +       } else {
> > +               unsigned long sp;
> > +               unsigned int rem;
> > +               u64 dyn_size;
> > +
> > +               /*
> > +                * Static size: we always dump the size
> > +                * requested by the user because most of the
> > +                * time, the top of the user stack is not
> > +                * paged out.
> > +                */
> > +               size = round_up(dump_size, sizeof(u64));
> 
> You also do this in the prepare thing..
> 
> > +               perf_output_put(handle, size);
> > +
> > +               sp = user_stack_pointer(regs);
> > +               rem = __output_copy_user(handle, (void *)sp, size);
> > +               dyn_size = size - rem;
> > +
> > +               /* What couldn't be dumped is zero padded */
> > +               while (rem--) {
> > +                       char zero = 0;
> > +                       perf_output_put(handle, zero);
> > +               }
> 
> Does this matter? If we don't write it the worst that can happen is that
> we leave previous ring-bugger content around, but since we already are
> privileged to read that (and very likely already have) there's no
> problem with that..
> 
> I know not zero-ing is ugly, but its also faster.. and do we care about
> them silly zeros?

The problem there is that the unwinder may then deal with random values
from previous records in the buffer and think it is some real stack values.
This may give strange results. At least zeroing was limiting this effect.

I think the best would be to do like you say: if we can't dump everything
then just leave the state of the buffer as is. But also output in the end
the effective size of the stack that has been dumped. So that we know what
we can give to the unwinder and what we can not.
--
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/