Date: Mon, 14 Aug 2017 16:22:11 +0900
From: Byungchul Park <byungchul.park@lge.com>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: Byungchul Park <max.byungchul.park@gmail.com>,
        Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@kernel.org>,
        tglx@linutronix.de, Michel Lespinasse <walken@google.com>,
        kirill@shutemov.name,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        linux-mm@kvack.org, akpm@linux-foundation.org, willy@infradead.org,
        npiggin@gmail.com, kernel-team@lge.com
Subject: Re: [PATCH v8 06/14] lockdep: Detect and handle hist_lock ring
 buffer overwrite
Message-ID: <20170814072211.GK20323@X58A-UD3R>
References: <20170810115922.kegrfeg6xz7mgpj4@tardis>
 <016b01d311d1$d02acfa0$70806ee0$@lge.com>
 <20170810125133.2poixhni4d5aqkpy@tardis>
 <20170810131737.skdyy4qcxlikbyeh@tardis>
 <20170811034328.GH20323@X58A-UD3R>
 <20170811080329.3ehu7pp7lcm62ji6@tardis>
 <20170811085201.GI20323@X58A-UD3R>
 <20170811094448.GJ20323@X58A-UD3R>
 <CANrsvRM4ijD0ym0HJySqjOfcCeUbGCc6bBppK43y5MqC5aB1gQ@mail.gmail.com>
 <20170814070522.wwj4as2hk2o7avlu@tardis>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170814070522.wwj4as2hk2o7avlu@tardis>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1169
Lines: 36

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 think Peterz's
> > approach should be modified a bit to make it work neatly, like:
> > 
> > crossrelease_hist_end(...)
> > {
> > ...
> >        invalidate_xhlock(&xhlock(cur->xhlock_idx_max));
> > 
> >        for (c = 0; c < XHLOCK_CXT_NR; c++)
> >               if ((cur->xhlock_idx_max - cur->xhlock_idx_hist[c]) >=
> > MAX_XHLOCKS_NR)
> >                      invalidate_xhlock(&xhlock(cur->xhlock_idx_hist[c]));
> > ...
> > }
> > 
> 
> Haven't looked into this deeply, but my gut feeling is this is
> unnecessary, will have a deep look.

Of course, for now, it looks like we can rely on the check_same_context()
on the commit, without invalidating it. But I think the approach might be
dangerous in future. I think it would be better to do it explicitlly.

> 
> Regards,
> Boqun
> 
> > And then Peterz's approach can also work, I think.
> > 
> > ---
> > Thanks,
> > Byungchul