Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 13 Jul 2017 11:50:52 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v7 06/16] lockdep: Detect and handle hist_lock ring buffer overwrite |
| |
On Thu, Jul 13, 2017 at 05:57:46PM +0900, Byungchul Park wrote: > On Thu, Jul 13, 2017 at 10:14:42AM +0200, Peter Zijlstra wrote: > > On Thu, Jul 13, 2017 at 11:07:45AM +0900, Byungchul Park wrote: > > > Does my approach have problems, rewinding to 'original idx' on exit and > > > deciding whether overwrite or not? I think, this way, no need to do the > > > drastic work. Or.. does my one get more overhead in usual case? > > > > So I think that invalidating just the one entry doesn't work; the moment > > I think invalidating just the one is enough. After rewinding, the entry > will be invalidated and the ring buffer starts to be filled forward from > the point with valid ones. When commit, it will proceed backward with > valid ones until meeting the invalidated entry and stop. > > IOW, in case of (overwritten) > > rewind to here > | > ppppppppppiiiiiiiiiiiiiiii > iiiiiiiiiiiiiii > > invalidate it on exit_irq > and start to fill from here again > | > pppppppppxiiiiiiiiiiiiiiii > iiiiiiiiiiiiiii > > when commit occurs here > | > pppppppppxpppppppppppiiiii > > do commit within this range > |<---------| > pppppppppxpppppppppppiiiii > > So I think this works and is much simple. Anything I missed?
wait_for_completion(&C); atomic_inc_return();
mutex_lock(A1); mutex_unlock(A1);
<IRQ> spin_lock(B1); spin_unlock(B1);
...
spin_lock(B64); spin_unlock(B64); </IRQ>
mutex_lock(A2); mutex_unlock(A2);
complete(&C);
That gives:
xhist[ 0] = A1 xhist[ 1] = B1 ... xhist[63] = B63
then we wrap and have:
xhist[0] = B64
then we rewind to 1 and invalidate to arrive at:
xhist[ 0] = B64 xhist[ 1] = NULL <-- idx xhist[ 2] = B2 ... xhist[63] = B63
Then we do A2 and get
xhist[ 0] = B64 xhist[ 1] = A2 <-- idx xhist[ 2] = B2 ... xhist[63] = B63
and the commit_xhlocks() will happily create links between C and A2, B2..B64.
The C<->A2 link is desired, the C<->B* are not.
| ![\](/images/icornerr.gif) |