| Message ID | 1470909022-687-1-git-send-email-johannes@sipsolutions.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > --- > 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
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 >
> Initial testing says that the change below must precede the change > to the definition of lockless_dereference(), so the two should go > together. Indeed. > 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. I don't mind hugely since I have a fix now, but this one actually causes >1K warnings (including some "too many warnings!") for my build (net/wireless and net/mac80211 only!) and drowns out the real ones... I'm sure other parts of the tree are similarly affected. johannes
On Thu, Aug 11, 2016 at 11:26:47AM -0700, Paul E. McKenney wrote: > 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. I was planning to stuff them in tip/locking/urgent, so they'd end up in this release.
On Fri, Aug 12, 2016 at 08:25:43PM +0200, Peter Zijlstra wrote: > On Thu, Aug 11, 2016 at 11:26:47AM -0700, Paul E. McKenney wrote: > > 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. > > I was planning to stuff them in tip/locking/urgent, so they'd end up in > this release. They seem to work fine for me, so for both: Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Thanx, Paul
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) {