[PATCH] drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[PATCH] drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[Chris Wilson]

To acquire all modeset locks requires a ww_ctx to be allocated. As this
is the legacy path and the allocation small, to reduce the changes
required (and complex untested error handling) to the legacy drivers, we
simply assume that the allocation succeeds. At present, it relies on the
too-small-to-fail rule, but syzbot found that by injecting a failure
here we would hit the WARN. Document that this allocation must succeed
with __GFP_NOFAIL.

Reported-by: syzbot (syzkaller)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_modeset_lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index e123497da0ca..963e23db0fe7 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -93,7 +93,7 @@ void drm_modeset_lock_all(struct drm_device *dev)
 	struct drm_modeset_acquire_ctx *ctx;
 	int ret;
 
-	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_NOFAIL);
 	if (WARN_ON(!ctx))
 		return;
 
-- 
2.15.0.rc2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[Chris Wilson]

To acquire all modeset locks requires a ww_ctx to be allocated. As this
is the legacy path and the allocation small, to reduce the changes
required (and complex untested error handling) to the legacy drivers, we
simply assume that the allocation succeeds. At present, it relies on the
too-small-to-fail rule, but syzbot found that by injecting a failure
here we would hit the WARN. Document that this allocation must succeed
with __GFP_NOFAIL.

Reported-by: syzbot (syzkaller)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Resent to the right list!
---
 drivers/gpu/drm/drm_modeset_lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index e123497da0ca..963e23db0fe7 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -93,7 +93,7 @@ void drm_modeset_lock_all(struct drm_device *dev)
 	struct drm_modeset_acquire_ctx *ctx;
 	int ret;
 
-	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_NOFAIL);
 	if (WARN_ON(!ctx))
 		return;
 
-- 
2.15.0.rc2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all (rev2)

[Patchwork]

== Series Details ==

Series: drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all (rev2)
URL   : https://patchwork.freedesktop.org/series/32899/
State : failure

== Summary ==

Series 32899v2 drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all
https://patchwork.freedesktop.org/api/1.0/series/32899/revisions/2/mbox/

Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-WARN (fi-skl-6700k)
Test kms_sink_crc_basic:
                skip       -> INCOMPLETE (fi-skl-6700k)
Test pm_rpm:
        Subgroup basic-rte:
                skip       -> PASS       (fi-hsw-4770r)
Test drv_module_reload:
        Subgroup basic-reload-inject:
                incomplete -> DMESG-WARN (fi-cfl-s) fdo#103206

fdo#103206 https://bugs.freedesktop.org/show_bug.cgi?id=103206

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:437s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:447s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:373s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:531s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:265s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:497s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:488s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:490s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:480s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:551s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:615s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:423s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:248s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:573s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:483s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:432s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:427s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:420s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:493s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:486s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:568s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:578s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:543s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:591s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:642s
fi-skl-6700k     total:250  pass:227  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:509s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:460s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:559s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:418s

dfe1410689e638c987b39e07288bc1951cb252f3 drm-tip: 2017y-10m-31d-09h-42m-59s UTC integration manifest
d99ca4a2b64d drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6278/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[Chris Wilson]

Quoting Chris Wilson (2017-10-31 11:55:35)
> To acquire all modeset locks requires a ww_ctx to be allocated. As this
> is the legacy path and the allocation small, to reduce the changes
> required (and complex untested error handling) to the legacy drivers, we
> simply assume that the allocation succeeds. At present, it relies on the
> too-small-to-fail rule, but syzbot found that by injecting a failure
> here we would hit the WARN. Document that this allocation must succeed
> with __GFP_NOFAIL.
> 
> Reported-by: syzbot (syzkaller)

Proper credit is
Reported-by: syzbot <syzkaller@googlegroups.com>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[Ville Syrjälä]

On Tue, Oct 31, 2017 at 11:55:35AM +0000, Chris Wilson wrote:
> To acquire all modeset locks requires a ww_ctx to be allocated. As this
> is the legacy path and the allocation small, to reduce the changes
> required (and complex untested error handling) to the legacy drivers, we
> simply assume that the allocation succeeds. At present, it relies on the
> too-small-to-fail rule, but syzbot found that by injecting a failure
> here we would hit the WARN. Document that this allocation must succeed
> with __GFP_NOFAIL.
> 
> Reported-by: syzbot (syzkaller)
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
> Resent to the right list!
> ---
>  drivers/gpu/drm/drm_modeset_lock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> index e123497da0ca..963e23db0fe7 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -93,7 +93,7 @@ void drm_modeset_lock_all(struct drm_device *dev)
>  	struct drm_modeset_acquire_ctx *ctx;
>  	int ret;
>  
> -	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_NOFAIL);
>  	if (WARN_ON(!ctx))
>  		return;
>  
> -- 
> 2.15.0.rc2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[Daniel Vetter]

On Tue, Oct 31, 2017 at 03:28:01PM +0200, Ville Syrjälä wrote:
> On Tue, Oct 31, 2017 at 11:55:35AM +0000, Chris Wilson wrote:
> > To acquire all modeset locks requires a ww_ctx to be allocated. As this
> > is the legacy path and the allocation small, to reduce the changes
> > required (and complex untested error handling) to the legacy drivers, we
> > simply assume that the allocation succeeds. At present, it relies on the
> > too-small-to-fail rule, but syzbot found that by injecting a failure
> > here we would hit the WARN. Document that this allocation must succeed
> > with __GFP_NOFAIL.

Note that for atomic drivers at least all the core/helper paths are fixed
up correctly. But e.g. i915 has still plenty of callsites in its own code,
mostly debugfs.

> > Reported-by: syzbot (syzkaller)
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Applied, thanks.
-Daniel

> 
> > ---
> > Resent to the right list!
> > ---
> >  drivers/gpu/drm/drm_modeset_lock.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> > index e123497da0ca..963e23db0fe7 100644
> > --- a/drivers/gpu/drm/drm_modeset_lock.c
> > +++ b/drivers/gpu/drm/drm_modeset_lock.c
> > @@ -93,7 +93,7 @@ void drm_modeset_lock_all(struct drm_device *dev)
> >  	struct drm_modeset_acquire_ctx *ctx;
> >  	int ret;
> >  
> > -	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> > +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_NOFAIL);
> >  	if (WARN_ON(!ctx))
> >  		return;
> >  
> > -- 
> > 2.15.0.rc2
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
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

Re: [PATCH] drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[Chris Wilson]

Quoting Daniel Vetter (2017-10-31 16:38:26)
> On Tue, Oct 31, 2017 at 03:28:01PM +0200, Ville Syrjälä wrote:
> > On Tue, Oct 31, 2017 at 11:55:35AM +0000, Chris Wilson wrote:
> > > To acquire all modeset locks requires a ww_ctx to be allocated. As this
> > > is the legacy path and the allocation small, to reduce the changes
> > > required (and complex untested error handling) to the legacy drivers, we
> > > simply assume that the allocation succeeds. At present, it relies on the
> > > too-small-to-fail rule, but syzbot found that by injecting a failure
> > > here we would hit the WARN. Document that this allocation must succeed
> > > with __GFP_NOFAIL.
> 
> Note that for atomic drivers at least all the core/helper paths are fixed
> up correctly. But e.g. i915 has still plenty of callsites in its own code,
> mostly debugfs.
> 
> > > Reported-by: syzbot (syzkaller)
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Applied, thanks.

Just curious as it hasn't shown up in drm-tip yet, so I'm worrying if it
found a crack to hide in.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[Daniel Vetter]

On Wed, Nov 01, 2017 at 02:08:37PM +0000, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-10-31 16:38:26)
> > On Tue, Oct 31, 2017 at 03:28:01PM +0200, Ville Syrjälä wrote:
> > > On Tue, Oct 31, 2017 at 11:55:35AM +0000, Chris Wilson wrote:
> > > > To acquire all modeset locks requires a ww_ctx to be allocated. As this
> > > > is the legacy path and the allocation small, to reduce the changes
> > > > required (and complex untested error handling) to the legacy drivers, we
> > > > simply assume that the allocation succeeds. At present, it relies on the
> > > > too-small-to-fail rule, but syzbot found that by injecting a failure
> > > > here we would hit the WARN. Document that this allocation must succeed
> > > > with __GFP_NOFAIL.
> > 
> > Note that for atomic drivers at least all the core/helper paths are fixed
> > up correctly. But e.g. i915 has still plenty of callsites in its own code,
> > mostly debugfs.
> > 
> > > > Reported-by: syzbot (syzkaller)
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > 
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Applied, thanks.
> 
> Just curious as it hasn't shown up in drm-tip yet, so I'm worrying if it
> found a crack to hide in.

Indeed it found a crack :-/

Pushed now, thanks for reminding me.
-Daniel
-- 
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