* [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
@ 2016-08-11 9:50 Johannes Berg
2016-08-11 9:50 ` [PATCH 2/2] locking/barriers: suppress sparse warnings in lockless_dereference() Johannes Berg
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Johannes Berg @ 2016-08-11 9:50 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul E . McKenney, Ingo Molnar, Chris Wilson,
Daniel Vetter, dri-devel, intel-gfx, Johannes Berg
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>
---
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] locking/barriers: suppress sparse warnings in lockless_dereference()
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 ` 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 ` Daniel Vetter
` (3 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Johannes Berg @ 2016-08-11 9:50 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Paul E . McKenney, Ingo Molnar, Chris Wilson,
Daniel Vetter, dri-devel, intel-gfx, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
After Peter's commit (see below) we get a lot of sparse warnings
(one for every rcu_dereference, and more) since the expression
here is assigning to the wrong address space.
Instead of validating that 'p' is a pointer this way, instead make
it fail compilation when it's not by using sizeof(*(p)). This will
not cause any sparse warnings (tested, likely since the address
space is irrelevant for sizeof), and will fail compilation when
'p' isn't a pointer type.
Fixes: 331b6d8c7afc ("locking/barriers: Validate lockless_dereference() is used on a pointer type")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/linux/compiler.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 1bb954842725..436aa4e42221 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -527,13 +527,13 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
* object's lifetime is managed by something other than RCU. That
* "something other" might be reference counting or simple immortality.
*
- * The seemingly unused void * variable is to validate @p is indeed a pointer
- * type. All pointer types silently cast to void *.
+ * The seemingly unused size_t variable is to validate @p is indeed a pointer
+ * type by making sure it can be dereferenced.
*/
#define lockless_dereference(p) \
({ \
typeof(p) _________p1 = READ_ONCE(p); \
- __maybe_unused const void * const _________p2 = _________p1; \
+ size_t __maybe_unused __size_of_ptr = sizeof(*(p)); \
smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
(_________p1); \
})
--
2.8.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
2016-08-11 9:50 [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference" Johannes Berg
@ 2016-08-11 10:38 ` Daniel Vetter
2016-08-11 10:38 ` Daniel Vetter
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2016-08-11 10:38 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-kernel, Johannes Berg, Peter Zijlstra, intel-gfx,
dri-devel, Daniel Vetter, Paul E . McKenney, Ingo Molnar
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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
@ 2016-08-11 10:38 ` Daniel Vetter
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2016-08-11 10:38 UTC (permalink / raw)
To: Johannes Berg
Cc: Johannes Berg, Peter Zijlstra, intel-gfx, linux-kernel,
dri-devel, Daniel Vetter, Paul E . McKenney, Ingo Molnar
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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* ✗ Ro.CI.BAT: failure for series starting with [1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
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-11 10:38 ` Daniel Vetter
@ 2016-08-11 11:29 ` 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
4 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2016-08-11 11:29 UTC (permalink / raw)
To: Johannes Berg; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
URL : https://patchwork.freedesktop.org/series/10948/
State : failure
== Summary ==
Series 10948v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/10948/revisions/1/mbox
Test gem_exec_suspend:
Subgroup basic-s3:
incomplete -> DMESG-WARN (fi-skl-i7-6700k)
Test kms_cursor_legacy:
Subgroup basic-flip-vs-cursor-legacy:
fail -> PASS (ro-hsw-i7-4770r)
fail -> PASS (ro-byt-n2820)
pass -> FAIL (fi-hsw-i7-4770k)
Subgroup basic-flip-vs-cursor-varying-size:
fail -> PASS (fi-hsw-i7-4770k)
fail -> PASS (ro-bdw-i5-5250u)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
dmesg-warn -> SKIP (ro-bdw-i5-5250u)
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> SKIP (ro-bdw-i7-5557U)
dmesg-warn -> SKIP (ro-bdw-i5-5250u)
Subgroup suspend-read-crc-pipe-c:
dmesg-warn -> SKIP (ro-bdw-i5-5250u)
fi-hsw-i7-4770k total:244 pass:221 dwarn:0 dfail:0 fail:1 skip:22
fi-kbl-qkkr total:244 pass:185 dwarn:30 dfail:1 fail:1 skip:27
fi-skl-i5-6260u total:244 pass:224 dwarn:4 dfail:0 fail:2 skip:14
fi-skl-i7-6700k total:244 pass:208 dwarn:4 dfail:2 fail:2 skip:28
fi-snb-i7-2600 total:244 pass:202 dwarn:0 dfail:0 fail:0 skip:42
ro-bdw-i5-5250u total:240 pass:220 dwarn:1 dfail:0 fail:0 skip:19
ro-bdw-i7-5557U total:240 pass:220 dwarn:1 dfail:0 fail:0 skip:19
ro-bdw-i7-5600u total:240 pass:207 dwarn:0 dfail:0 fail:1 skip:32
ro-bsw-n3050 total:240 pass:194 dwarn:0 dfail:0 fail:4 skip:42
ro-byt-n2820 total:240 pass:199 dwarn:0 dfail:0 fail:1 skip:40
ro-hsw-i3-4010u total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26
ro-hsw-i7-4770r total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26
ro-ilk1-i5-650 total:235 pass:173 dwarn:0 dfail:0 fail:2 skip:60
ro-ivb-i7-3770 total:240 pass:205 dwarn:0 dfail:0 fail:0 skip:35
ro-ivb2-i7-3770 total:240 pass:209 dwarn:0 dfail:0 fail:0 skip:31
ro-skl3-i5-6260u total:240 pass:222 dwarn:0 dfail:0 fail:4 skip:14
Results at /archive/results/CI_IGT_test/RO_Patchwork_1835/
3c6050a drm-intel-nightly: 2016y-08m-11d-10h-34m-38s UTC integration manifest
8ff8d08 locking/barriers: suppress sparse warnings in lockless_dereference()
fcc9432 Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
2016-08-11 10:38 ` Daniel Vetter
@ 2016-08-11 18:26 ` Paul E. McKenney
-1 siblings, 0 replies; 16+ messages in thread
From: Paul E. McKenney @ 2016-08-11 18:26 UTC (permalink / raw)
To: Johannes Berg, linux-kernel, Johannes Berg, Peter Zijlstra,
intel-gfx, dri-devel, Daniel Vetter, Ingo Molnar
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
@ 2016-08-11 18:26 ` Paul E. McKenney
0 siblings, 0 replies; 16+ messages in thread
From: Paul E. McKenney @ 2016-08-11 18:26 UTC (permalink / raw)
To: Johannes Berg, linux-kernel, Johannes Berg, Peter Zijlstra,
intel-gfx, dri-devel, Daniel Vetter, Ingo Molnar
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
2016-08-11 18:26 ` Paul E. McKenney
@ 2016-08-12 6:05 ` Johannes Berg
-1 siblings, 0 replies; 16+ messages in thread
From: Johannes Berg @ 2016-08-12 6:05 UTC (permalink / raw)
To: paulmck, linux-kernel, Peter Zijlstra, intel-gfx, dri-devel,
Daniel Vetter, Ingo Molnar
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
@ 2016-08-12 6:05 ` Johannes Berg
0 siblings, 0 replies; 16+ messages in thread
From: Johannes Berg @ 2016-08-12 6:05 UTC (permalink / raw)
To: paulmck, linux-kernel, Peter Zijlstra, intel-gfx, dri-devel,
Daniel Vetter, Ingo Molnar
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
2016-08-11 18:26 ` Paul E. McKenney
(?)
(?)
@ 2016-08-12 18:25 ` Peter Zijlstra
2016-08-12 19:15 ` Paul E. McKenney
-1 siblings, 1 reply; 16+ messages in thread
From: Peter Zijlstra @ 2016-08-12 18:25 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Johannes Berg, linux-kernel, Johannes Berg, intel-gfx, dri-devel,
Daniel Vetter, Ingo Molnar
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
2016-08-12 18:25 ` Peter Zijlstra
@ 2016-08-12 19:15 ` Paul E. McKenney
0 siblings, 0 replies; 16+ messages in thread
From: Paul E. McKenney @ 2016-08-12 19:15 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Johannes Berg, linux-kernel, Johannes Berg, intel-gfx, dri-devel,
Daniel Vetter, Ingo Molnar
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
@ 2016-08-12 19:15 ` Paul E. McKenney
0 siblings, 0 replies; 16+ messages in thread
From: Paul E. McKenney @ 2016-08-12 19:15 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Johannes Berg, intel-gfx, linux-kernel, dri-devel, Daniel Vetter,
Johannes Berg, Ingo Molnar
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [tip:locking/core] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
2016-08-11 9:50 [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference" Johannes Berg
` (2 preceding siblings ...)
2016-08-11 11:29 ` ✗ Ro.CI.BAT: failure for series starting with [1/2] " Patchwork
@ 2016-08-18 10:59 ` tip-bot for Johannes Berg
2016-08-18 13:40 ` [tip:locking/urgent] " tip-bot for Johannes Berg
4 siblings, 0 replies; 16+ messages in thread
From: tip-bot for Johannes Berg @ 2016-08-18 10:59 UTC (permalink / raw)
To: linux-tip-commits
Cc: peterz, paulmck, mingo, tglx, chris, akpm, hpa, johannes.berg,
torvalds, linux-kernel, daniel.vetter
Commit-ID: 8180cfb96098f7066fbd0a44ac3aaf422f74609d
Gitweb: http://git.kernel.org/tip/8180cfb96098f7066fbd0a44ac3aaf422f74609d
Author: Johannes Berg <johannes.berg@intel.com>
AuthorDate: Thu, 11 Aug 2016 11:50:21 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 18 Aug 2016 11:34:26 +0200
Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
This reverts commit:
fa7d81bb3c269 ("drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference")
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 the compile without this revert, because it assumes
you want to deference and thus also need the struct type visible (which
it isn't here), so revert it.
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1470909022-687-1-git-send-email-johannes@sipsolutions.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
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 ce54e98..0a06f91 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) {
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [tip:locking/core] locking/barriers: Suppress sparse warnings in lockless_dereference()
2016-08-11 9:50 ` [PATCH 2/2] locking/barriers: suppress sparse warnings in lockless_dereference() Johannes Berg
@ 2016-08-18 11:00 ` tip-bot for Johannes Berg
2016-08-18 13:40 ` [tip:locking/urgent] " tip-bot for Johannes Berg
1 sibling, 0 replies; 16+ messages in thread
From: tip-bot for Johannes Berg @ 2016-08-18 11:00 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, peterz, mingo, akpm, tglx, torvalds, daniel.vetter,
chris, paulmck, johannes.berg, hpa
Commit-ID: 9054b5958ca3cc83ec815d40ed5b5176bf422a03
Gitweb: http://git.kernel.org/tip/9054b5958ca3cc83ec815d40ed5b5176bf422a03
Author: Johannes Berg <johannes.berg@intel.com>
AuthorDate: Thu, 11 Aug 2016 11:50:22 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 18 Aug 2016 11:34:27 +0200
locking/barriers: Suppress sparse warnings in lockless_dereference()
After Peter's commit:
331b6d8c7afc ("locking/barriers: Validate lockless_dereference() is used on a pointer type")
... we get a lot of sparse warnings (one for every rcu_dereference, and more)
since the expression here is assigning to the wrong address space.
Instead of validating that 'p' is a pointer this way, instead make
it fail compilation when it's not by using sizeof(*(p)). This will
not cause any sparse warnings (tested, likely since the address
space is irrelevant for sizeof), and will fail compilation when
'p' isn't a pointer type.
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 331b6d8c7afc ("locking/barriers: Validate lockless_dereference() is used on a pointer type")
Link: http://lkml.kernel.org/r/1470909022-687-2-git-send-email-johannes@sipsolutions.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/compiler.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 1bb9548..436aa4e 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -527,13 +527,13 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
* object's lifetime is managed by something other than RCU. That
* "something other" might be reference counting or simple immortality.
*
- * The seemingly unused void * variable is to validate @p is indeed a pointer
- * type. All pointer types silently cast to void *.
+ * The seemingly unused size_t variable is to validate @p is indeed a pointer
+ * type by making sure it can be dereferenced.
*/
#define lockless_dereference(p) \
({ \
typeof(p) _________p1 = READ_ONCE(p); \
- __maybe_unused const void * const _________p2 = _________p1; \
+ size_t __maybe_unused __size_of_ptr = sizeof(*(p)); \
smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
(_________p1); \
})
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [tip:locking/urgent] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
2016-08-11 9:50 [PATCH 1/2] Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference" Johannes Berg
` (3 preceding siblings ...)
2016-08-18 10:59 ` [tip:locking/core] " tip-bot for Johannes Berg
@ 2016-08-18 13:40 ` tip-bot for Johannes Berg
4 siblings, 0 replies; 16+ messages in thread
From: tip-bot for Johannes Berg @ 2016-08-18 13:40 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, johannes.berg, mingo, daniel.vetter, hpa, tglx,
chris, akpm, peterz, torvalds, paulmck
Commit-ID: f17b3ea3d2df7c9bf3ce1dbd65b5fd7061f8e787
Gitweb: http://git.kernel.org/tip/f17b3ea3d2df7c9bf3ce1dbd65b5fd7061f8e787
Author: Johannes Berg <johannes.berg@intel.com>
AuthorDate: Thu, 11 Aug 2016 11:50:21 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 18 Aug 2016 15:36:13 +0200
Revert "drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference"
This reverts commit:
fa7d81bb3c269 ("drm/fb-helper: Reduce READ_ONCE(master) to lockless_dereference")
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 the compile without this revert, because it assumes
you want to deference and thus also need the struct type visible (which
it isn't here), so revert it.
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1470909022-687-1-git-send-email-johannes@sipsolutions.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
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 ce54e98..0a06f91 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) {
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [tip:locking/urgent] locking/barriers: Suppress sparse warnings in lockless_dereference()
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-bot for Johannes Berg
1 sibling, 0 replies; 16+ messages in thread
From: tip-bot for Johannes Berg @ 2016-08-18 13:40 UTC (permalink / raw)
To: linux-tip-commits
Cc: johannes.berg, akpm, tglx, peterz, mingo, daniel.vetter, hpa,
paulmck, torvalds, linux-kernel, chris
Commit-ID: 112dc0c8069e5554e0ad29c58228f1e6ca49e13d
Gitweb: http://git.kernel.org/tip/112dc0c8069e5554e0ad29c58228f1e6ca49e13d
Author: Johannes Berg <johannes.berg@intel.com>
AuthorDate: Thu, 11 Aug 2016 11:50:22 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 18 Aug 2016 15:36:13 +0200
locking/barriers: Suppress sparse warnings in lockless_dereference()
After Peter's commit:
331b6d8c7afc ("locking/barriers: Validate lockless_dereference() is used on a pointer type")
... we get a lot of sparse warnings (one for every rcu_dereference, and more)
since the expression here is assigning to the wrong address space.
Instead of validating that 'p' is a pointer this way, instead make
it fail compilation when it's not by using sizeof(*(p)). This will
not cause any sparse warnings (tested, likely since the address
space is irrelevant for sizeof), and will fail compilation when
'p' isn't a pointer type.
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 331b6d8c7afc ("locking/barriers: Validate lockless_dereference() is used on a pointer type")
Link: http://lkml.kernel.org/r/1470909022-687-2-git-send-email-johannes@sipsolutions.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/compiler.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 1bb9548..436aa4e 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -527,13 +527,13 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
* object's lifetime is managed by something other than RCU. That
* "something other" might be reference counting or simple immortality.
*
- * The seemingly unused void * variable is to validate @p is indeed a pointer
- * type. All pointer types silently cast to void *.
+ * The seemingly unused size_t variable is to validate @p is indeed a pointer
+ * type by making sure it can be dereferenced.
*/
#define lockless_dereference(p) \
({ \
typeof(p) _________p1 = READ_ONCE(p); \
- __maybe_unused const void * const _________p2 = _________p1; \
+ size_t __maybe_unused __size_of_ptr = sizeof(*(p)); \
smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
(_________p1); \
})
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-08-18 13:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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.