[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

✓ Fi.CI.BAT: success for drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all

[Patchwork]

== Series Details ==

Series: drm: Require __GFP_NOFAIL for the legacy drm_modeset_lock_all
URL   : https://patchwork.freedesktop.org/series/32899/
State : success

== Summary ==

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

Test pm_rpm:
        Subgroup basic-rte:
                skip       -> PASS       (fi-hsw-4770r) fdo#103522
Test drv_module_reload:
        Subgroup basic-reload-inject:
                incomplete -> DMESG-WARN (fi-cfl-s) fdo#103206

fdo#103522 https://bugs.freedesktop.org/show_bug.cgi?id=103522
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:440s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:448s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:371s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:529s
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:502s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:498s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:493s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:479s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:556s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:606s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:256s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:583s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:482s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:431s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:434s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:421s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:490s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:464s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:491s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:573s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:474s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:589s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:457s
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:644s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:526s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:452s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:564s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:417s

dfe1410689e638c987b39e07288bc1951cb252f3 drm-tip: 2017y-10m-31d-09h-42m-59s UTC integration manifest
e1be47442043 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_6277/
_______________________________________________
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 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

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

[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

[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

[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