From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Johannes Berg <johannes@sipsolutions.net>,
linux-kernel@vger.kernel.org,
Johannes Berg <johannes.berg@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
Daniel Vetter <daniel.vetter@intel.com>,
Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
Date: Thu, 11 Aug 2016 11:26:47 -0700 [thread overview]
Message-ID: <20160811182647.GD3482@linux.vnet.ibm.com> (raw)
In-Reply-To: <20160811103859.GL6232@phenom.ffwll.local>
On Thu, Aug 11, 2016 at 12:38:59PM +0200, Daniel Vetter wrote:
> On Thu, Aug 11, 2016 at 11:50:21AM +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > This reverts commit fa7d81bb3c269a2ee38b6e4d569d9eb8be1a78ad.
> >
> > As Peter explained:
> > [...] lockless_dereference() is _stronger_ than READ_ONCE(), not weaker.
> >
> > [...]
> >
> > Also, clue is in the name: 'dereference', you don't actually dereference
> > the pointer here, only load it.
> >
> > My next patch breaks compile on this, because it assumes you want to
> > deference and thus also need the struct type visible (which it isn't
> > here), so revert it.
> >
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> And ack-by: me for merging through whatever tree this makes sense for.
> -Daniel
Initial testing says that the change below must precede the change
to the definition of lockless_dereference(), so the two should go
together.
If my upcoming testing of the two changes together pans out, I will
give you a Tested-by -- I am guessing that you don't want to wait
until the next merge window for these changes.
Thanx, Paul
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index ce54e985d91b..0a06f9120b5a 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ 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 (lockless_dereference(dev->master))
> > + if (READ_ONCE(dev->master))
> > return false;
> >
> > drm_for_each_crtc(crtc, dev) {
> > --
> > 2.8.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
WARNING: multiple messages have this Message-ID (diff)
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Johannes Berg <johannes@sipsolutions.net>,
linux-kernel@vger.kernel.org,
Johannes Berg <johannes.berg@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
Daniel Vetter <daniel.vetter@intel.com>,
Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
Date: Thu, 11 Aug 2016 11:26:47 -0700 [thread overview]
Message-ID: <20160811182647.GD3482@linux.vnet.ibm.com> (raw)
In-Reply-To: <20160811103859.GL6232@phenom.ffwll.local>
On Thu, Aug 11, 2016 at 12:38:59PM +0200, Daniel Vetter wrote:
> On Thu, Aug 11, 2016 at 11:50:21AM +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > This reverts commit fa7d81bb3c269a2ee38b6e4d569d9eb8be1a78ad.
> >
> > As Peter explained:
> > [...] lockless_dereference() is _stronger_ than READ_ONCE(), not weaker.
> >
> > [...]
> >
> > Also, clue is in the name: 'dereference', you don't actually dereference
> > the pointer here, only load it.
> >
> > My next patch breaks compile on this, because it assumes you want to
> > deference and thus also need the struct type visible (which it isn't
> > here), so revert it.
> >
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> And ack-by: me for merging through whatever tree this makes sense for.
> -Daniel
Initial testing says that the change below must precede the change
to the definition of lockless_dereference(), so the two should go
together.
If my upcoming testing of the two changes together pans out, I will
give you a Tested-by -- I am guessing that you don't want to wait
until the next merge window for these changes.
Thanx, Paul
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index ce54e985d91b..0a06f9120b5a 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ 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 (lockless_dereference(dev->master))
> > + if (READ_ONCE(dev->master))
> > return false;
> >
> > drm_for_each_crtc(crtc, dev) {
> > --
> > 2.8.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
next prev parent reply other threads:[~2016-08-11 18:26 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-11 9:50 [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference" Johannes Berg
2016-08-11 9:50 ` [PATCH 2/2] locking/barriers: suppress sparse warnings in lockless_dereference() Johannes Berg
2016-08-18 11:00 ` [tip:locking/core] locking/barriers: Suppress " tip-bot for Johannes Berg
2016-08-18 13:40 ` [tip:locking/urgent] " tip-bot for Johannes Berg
2016-08-11 10:38 ` [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference" Daniel Vetter
2016-08-11 10:38 ` Daniel Vetter
2016-08-11 18:26 ` Paul E. McKenney [this message]
2016-08-11 18:26 ` Paul E. McKenney
2016-08-12 6:05 ` Johannes Berg
2016-08-12 6:05 ` Johannes Berg
2016-08-12 18:25 ` Peter Zijlstra
2016-08-12 19:15 ` Paul E. McKenney
2016-08-12 19:15 ` Paul E. McKenney
2016-08-11 11:29 ` ✗ Ro.CI.BAT: failure for series starting with [1/2] " Patchwork
2016-08-18 10:59 ` [tip:locking/core] " tip-bot for Johannes Berg
2016-08-18 13:40 ` [tip:locking/urgent] " tip-bot for Johannes Berg
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=20160811182647.GD3482@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.com \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=johannes.berg@intel.com \
--cc=johannes@sipsolutions.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.