On Mon, Aug 14, 2017 at 03:05:22PM +0800, Boqun Feng wrote:
> > 1. Boqun's approach
>
> My approach requires(additionally):
>
> MAX_XHLOCKS_NR * sizeof(unsigned int) // because of the hist_id field
> in hist_lock
>
> bytes per task.
>
> > 2. Peterz's approach
>
> And Peter's approach re
On Mon, Aug 14, 2017 at 03:05:22PM +0800, Boqun Feng wrote:
> > I like Boqun's approach most but, _whatever_. It's ok if it solves the
> > problem.
> > The last one is not bad when it is used for syscall exit, but we have to
> > give
> > up valid dependencies unnecessarily in other cases. And I t
On Fri, Aug 11, 2017 at 10:06:37PM +0900, Byungchul Park wrote:
> On Fri, Aug 11, 2017 at 6:44 PM, Byungchul Park
> wrote:
> > On Fri, Aug 11, 2017 at 05:52:02PM +0900, Byungchul Park wrote:
> >> On Fri, Aug 11, 2017 at 04:03:29PM +0800, Boqun Feng wrote:
> >> > Thanks for taking a look at it ;-)
On Fri, Aug 11, 2017 at 6:44 PM, Byungchul Park wrote:
> On Fri, Aug 11, 2017 at 05:52:02PM +0900, Byungchul Park wrote:
>> On Fri, Aug 11, 2017 at 04:03:29PM +0800, Boqun Feng wrote:
>> > Thanks for taking a look at it ;-)
>>
>> I rather appriciate it.
>>
>> > > > @@ -5005,7 +5003,7 @@ static int
On Fri, Aug 11, 2017 at 05:52:02PM +0900, Byungchul Park wrote:
> On Fri, Aug 11, 2017 at 04:03:29PM +0800, Boqun Feng wrote:
> > Thanks for taking a look at it ;-)
>
> I rather appriciate it.
>
> > > > @@ -5005,7 +5003,7 @@ static int commit_xhlock(struct cross_lock
> > > > *xlock, struct hist_
On Fri, Aug 11, 2017 at 04:03:29PM +0800, Boqun Feng wrote:
> Thanks for taking a look at it ;-)
I rather appriciate it.
> > > @@ -5005,7 +5003,7 @@ static int commit_xhlock(struct cross_lock *xlock,
> > > struct hist_lock *xhlock)
> > > static void commit_xhlocks(struct cross_lock *xlock)
> >
On Fri, Aug 11, 2017 at 12:43:28PM +0900, Byungchul Park wrote:
> On Thu, Aug 10, 2017 at 09:17:37PM +0800, Boqun Feng wrote:
> > > > > > @@ -4826,6 +4851,7 @@ static inline int depend_after(struct
> > > > > > held_lock
> > > > > *hlock)
> > > > > > * Check if the xhlock is valid, which would be
On Thu, Aug 10, 2017 at 09:17:37PM +0800, Boqun Feng wrote:
> > > > > @@ -4826,6 +4851,7 @@ static inline int depend_after(struct held_lock
> > > > *hlock)
> > > > > * Check if the xhlock is valid, which would be false if,
> > > > > *
> > > > > *1. Has not used after initializaion yet.
>
On Fri, Aug 11, 2017 at 09:40:21AM +0900, Byungchul Park wrote:
> On Thu, Aug 10, 2017 at 08:51:33PM +0800, Boqun Feng wrote:
> > > > > void crossrelease_hist_end(enum context_t c)
> > > > > {
> > > > > - if (current->xhlocks)
> > > > > - current->xhlock_idx = current->xhlock_idx_
On Thu, Aug 10, 2017 at 09:17:37PM +0800, Boqun Feng wrote:
> So basically, I'm suggesting do this on top of your patch, there is also
> a fix in commit_xhlocks(), which I think you should swap the parameters
> in before(...), no matter using task_struct::hist_id or using
> task_struct::xhlock_idx
On Thu, Aug 10, 2017 at 08:51:33PM +0800, Boqun Feng wrote:
> > > > void crossrelease_hist_end(enum context_t c)
> > > > {
> > > > - if (current->xhlocks)
> > > > - current->xhlock_idx = current->xhlock_idx_hist[c];
> > > > + struct task_struct *cur = current;
> > > > +
On Thu, Aug 10, 2017 at 08:51:33PM +0800, Boqun Feng wrote:
[...]
> > > > + /* Check if the ring was overwritten. */
> > > > + if (h->hist_id != cur->hist_id_save[c])
> > >
> > > Could we use:
> > >
> > > if (h->hist_id != idx)
> >
> > No, we cannot.
> >
>
g; t...@linutronix.de;
> > wal...@google.com; kir...@shutemov.name; linux-kernel@vger.kernel.org;
> > linux...@kvack.org; a...@linux-foundation.org; wi...@infradead.org;
> > npig...@gmail.com; kernel-t...@lge.com
> > Subject: Re: [PATCH v8 06/14] lockdep: Detect and handle hist_lock r
ernel.org;
> linux...@kvack.org; a...@linux-foundation.org; wi...@infradead.org;
> npig...@gmail.com; kernel-t...@lge.com
> Subject: Re: [PATCH v8 06/14] lockdep: Detect and handle hist_lock ring
> buffer overwrite
>
> On Mon, Aug 07, 2017 at 04:12:53PM +0900, Byungchul Park wrote:
>
On Mon, Aug 07, 2017 at 04:12:53PM +0900, Byungchul Park wrote:
> The ring buffer can be overwritten by hardirq/softirq/work contexts.
> That cases must be considered on rollback or commit. For example,
>
> |<-- hist_lock ring buffer size ->|
> i
On Wed, Aug 09, 2017 at 04:16:05PM +0200, Peter Zijlstra wrote:
>
> Hehe, _another_ scheme...
>
> Yes I think this works.. but I had just sort of understood the last one.
>
> How about I do this on top? That I think is a combination of what I
> proposed last and your single invalidate thing. Com
On Thu, Aug 10, 2017 at 10:32:16AM +0900, Byungchul Park wrote:
> On Wed, Aug 09, 2017 at 04:16:05PM +0200, Peter Zijlstra wrote:
> > Hehe, _another_ scheme...
> >
> > Yes I think this works.. but I had just sort of understood the last one.
> >
> > How about I do this on top? That I think is a co
On Wed, Aug 09, 2017 at 04:16:05PM +0200, Peter Zijlstra wrote:
> Hehe, _another_ scheme...
>
> Yes I think this works.. but I had just sort of understood the last one.
>
> How about I do this on top? That I think is a combination of what I
> proposed last and your single invalidate thing. Combin
On Mon, Aug 07, 2017 at 04:12:53PM +0900, Byungchul Park wrote:
> @@ -4773,14 +4784,28 @@ void lockdep_rcu_suspicious(const char *file, const
> int line, const char *s)
> */
> void crossrelease_hist_start(enum context_t c)
> {
> - if (current->xhlocks)
> - current->xhlock_idx_h
The ring buffer can be overwritten by hardirq/softirq/work contexts.
That cases must be considered on rollback or commit. For example,
|<-- hist_lock ring buffer size ->|
iii
wrapped > iii
20 matches
Mail list logo
