From: Peter Zijlstra <peterz@infradead.org>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org,
Emil Velikov <emil.l.velikov@gmail.com>,
dri-devel@lists.freedesktop.org,
Julia Lawall <julia.lawall@lip6.fr>,
Daniel Vetter <daniel.vetter@intel.com>,
Johannes Berg <johannes@sipsolutions.net>,
Paul McKenney <paulmck@linux.vnet.ibm.com>
Subject: Re: drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference
Date: Thu, 11 Aug 2016 11:44:08 +0200 [thread overview]
Message-ID: <20160811094408.GA5845@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1466581572-16608-1-git-send-email-chris@chris-wilson.co.uk>
On Wed, Jun 22, 2016 at 08:46:12AM +0100, Chris Wilson wrote:
> We are only documenting that the read is outside of the lock, and do not
> require strict ordering on the operation. In this case the more relaxed
> lockless_dereference() will suffice.
No, no, no... This is 'broken'. lockless_dereference() is _stronger_
than READ_ONCE(), not weaker.
lockless_dereference() is a wrapper around smp_read_barrier_depends()
and is used to form read dependencies. There is no read dependency here,
therefore using lockless_dereference() is entirely pointless.
Look at the definition of lockless_dereference(), it does a READ_ONCE()
and then smp_read_barrier_depends().
Also, clue is in the name: 'dereference', you don't actually dereference
the pointer here, only load it.
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -464,7 +464,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
>
> /* Sometimes user space wants everything disabled, so don't steal the
> * display if there's a master. */
> - if (READ_ONCE(dev->master))
> + if (lockless_dereference(dev->master))
> return false;
>
> drm_for_each_crtc(crtc, dev) {
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2016-08-11 9:44 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-22 7:46 [PATCH] drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference Chris Wilson
2016-06-22 8:07 ` Daniel Vetter
2016-06-22 8:10 ` ✓ Ro.CI.BAT: success for " Patchwork
2016-08-11 9:44 ` Peter Zijlstra [this message]
2016-08-11 17:57 ` Paul E. McKenney
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=20160811094408.GA5845@twins.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=chris@chris-wilson.co.uk \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=emil.l.velikov@gmail.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=johannes@sipsolutions.net \
--cc=julia.lawall@lip6.fr \
--cc=paulmck@linux.vnet.ibm.com \
/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