mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>,
	johannes.berg@intel.com, mingo@kernel.org, tglx@linutronix.de,
	oleg@redhat.com, linux-kernel@vger.kernel.org,
	kernel-team@lge.com
Subject: Re: [RESEND PATCH v2 2/2] lockdep: Remove unnecessary acquisitions wrt workqueue flush
Date: Fri, 13 Oct 2017 16:56:33 +0900	[thread overview]
Message-ID: <20171013075633.GN3323@X58A-UD3R> (raw)
In-Reply-To: <20171012155635.ntljsbmtq4mstb2f@hirez.programming.kicks-ass.net>

On Thu, Oct 12, 2017 at 05:56:35PM +0200, Peter Zijlstra wrote:
> On Thu, Oct 12, 2017 at 08:38:17AM -0700, Tejun Heo wrote:
> > 
> > As long as we have the same level of protection, simpler code is of
> > course preferable.  That said, I haven't followed the discussion
> > closely and don't want to apply it without Peter's ack.  Peter?
> 
> I'm really tied up atm; and feel we should be addressing the false
> positives generated by the current code before we start doing new stuff
> on top.

We can never avoid adding false dependencies as long as we use
acquisitions in that way the workqueue code does, even though you
successfully replace write acquisitions with recursive-read ones after
making them work, as you know.

Speaking a bit more about the reason, it's because all write locks used
in every work->func() obviously generate false dependencies(links) with
'work' lockdep_map and 'wq' lockdep_map, when they do not involve flush.
This is why I used a word, 'speculative', whenever we were talking.

At the beginning of this issue, I suggested to use recursive-read
acquisitions instead of write ones, which you are working on, since
anyway it reduces false ones.

But, if it's allowed to add a new primitive that just suits that purpose,
I want to propose it to be used instead, which makes false ones reduced
more. A read acuisition is a real acquisition used for a read lock.
Semantics are similar to but not same as what we need for that
speculative purpose.

  reply	other threads:[~2017-10-13  7:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-10  6:51 [RESEND PATCH v2 1/2] completion: Add support for initializing completion with lockdep_map Byungchul Park
2017-10-10  6:51 ` [RESEND PATCH v2 2/2] lockdep: Remove unnecessary acquisitions wrt workqueue flush Byungchul Park
2017-10-12  7:54   ` Byungchul Park
2017-10-12 15:38     ` Tejun Heo
2017-10-12 15:56       ` Peter Zijlstra
2017-10-13  7:56         ` Byungchul Park [this message]
2017-10-13  8:27           ` Peter Zijlstra
2017-10-13  8:48             ` Byungchul Park
2017-10-16 10:58             ` Byungchul Park
2017-10-16 11:06               ` Byungchul Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171013075633.GN3323@X58A-UD3R \
    --to=byungchul.park@lge.com \
    --cc=johannes.berg@intel.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome