Re: commit b00bc0b237055b breaking perf subsystem

[Peter Zijlstra]

On Thu, 2009-11-19 at 11:50 -0800, Arjan van de Ven wrote:
> commit b00bc0b237055b4c45816325ee14f0bd83e6f590
> Author: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Date:   Mon Nov 2 13:01:56 2009 +0100
> 
>     uids: Prevent tear down race
> 
> has been bisected to break the perf system between -rc5 and -rc7;
> and reverting this patch in rc7 also fixes the issue.
> 
> now this patch is.... fun and seemingly unrelated to perf, so I
> am pretty sure something else is going on.
> 
> the symptom of the breakage is that a userland client of perf
> (in my case, powertop) does not get to see new events if there
> is only, say, one or two of them. The data_head pointer just does
> not get updated.
> 
> Based on the patch, I have a suspicion that this pointer gets updated
> from synchronize RCU context, and this patch just makes that happen
> more.
> 
> If that is the case, what we really need is a new ioctl to perf that
> will cause all pending buffer state to be flushed to the ring buffer,
> that applications using perf can then just always call before looking
> at the ring. As a concept, that sounds like a good idea to me anyway...
> 
> comments/suggestions/saying-I'm-full-of-sh*t ?

We should be updating the data_head pointer after every writen event
(with the exception of nested writes, where we delay publishing until
the last writer is done, so as to not expose incomplete data).

  perf_output_begin(); /* reserve space */
  perf_output_copy();  /* write bits to space */
  perf_output_end();   /* commit and publish */
    perf_output_unlock();
      /* complicated stuff to publish data_head */
      
Colour me puzzled on how this particular commit can have any effect on
data_head.

--
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/