[PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Jiasheng Jiang]

Add checks for the return value of alloc_workqueue and
alloc_ordered_workqueue as they may return NULL pointer and cause NULL
pointer dereference.
Moreover, destroy them when fails later in order to avoid memory leak.
Because in `drivers/gpu/drm/i915/i915_driver.c`, if
intel_modeset_init_noirq fails, its workqueues will not be destroyed.

Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6c2686ecb62a..58f6840d6bd8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 	intel_dmc_ucode_init(i915);
 
 	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
+	if (!i915->display.wq.modeset) {
+		ret = -ENOMEM;
+		goto cleanup_vga_client_pw_domain_dmc;
+	}
+
 	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
 						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
+	if (!i915->display.wq.flip) {
+		ret = -ENOMEM;
+		goto cleanup_modeset;
+	}
 
 	intel_mode_config_init(i915);
 
 	ret = intel_cdclk_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_color_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_dbuf_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_bw_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	init_llist_head(&i915->display.atomic_helper.free_list);
 	INIT_WORK(&i915->display.atomic_helper.free_work,
@@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 
 	return 0;
 
+cleanup_flip:
+	destroy_workqueue(i915->display.wq.flip);
+cleanup_modeset:
+	destroy_workqueue(i915->display.wq.modeset);
 cleanup_vga_client_pw_domain_dmc:
 	intel_dmc_ucode_fini(i915);
 	intel_power_domains_driver_remove(i915);
-- 
2.25.1

[PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Jiasheng Jiang]

Add checks for the return value of alloc_workqueue and
alloc_ordered_workqueue as they may return NULL pointer and cause NULL
pointer dereference.
Moreover, destroy them when fails later in order to avoid memory leak.
Because in `drivers/gpu/drm/i915/i915_driver.c`, if
intel_modeset_init_noirq fails, its workqueues will not be destroyed.

Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6c2686ecb62a..58f6840d6bd8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 	intel_dmc_ucode_init(i915);
 
 	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
+	if (!i915->display.wq.modeset) {
+		ret = -ENOMEM;
+		goto cleanup_vga_client_pw_domain_dmc;
+	}
+
 	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
 						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
+	if (!i915->display.wq.flip) {
+		ret = -ENOMEM;
+		goto cleanup_modeset;
+	}
 
 	intel_mode_config_init(i915);
 
 	ret = intel_cdclk_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_color_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_dbuf_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_bw_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	init_llist_head(&i915->display.atomic_helper.free_list);
 	INIT_WORK(&i915->display.atomic_helper.free_work,
@@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 
 	return 0;
 
+cleanup_flip:
+	destroy_workqueue(i915->display.wq.flip);
+cleanup_modeset:
+	destroy_workqueue(i915->display.wq.modeset);
 cleanup_vga_client_pw_domain_dmc:
 	intel_dmc_ucode_fini(i915);
 	intel_power_domains_driver_remove(i915);
-- 
2.25.1

[Intel-gfx] [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Jiasheng Jiang]

Add checks for the return value of alloc_workqueue and
alloc_ordered_workqueue as they may return NULL pointer and cause NULL
pointer dereference.
Moreover, destroy them when fails later in order to avoid memory leak.
Because in `drivers/gpu/drm/i915/i915_driver.c`, if
intel_modeset_init_noirq fails, its workqueues will not be destroyed.

Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6c2686ecb62a..58f6840d6bd8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 	intel_dmc_ucode_init(i915);
 
 	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
+	if (!i915->display.wq.modeset) {
+		ret = -ENOMEM;
+		goto cleanup_vga_client_pw_domain_dmc;
+	}
+
 	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
 						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
+	if (!i915->display.wq.flip) {
+		ret = -ENOMEM;
+		goto cleanup_modeset;
+	}
 
 	intel_mode_config_init(i915);
 
 	ret = intel_cdclk_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_color_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_dbuf_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	ret = intel_bw_init(i915);
 	if (ret)
-		goto cleanup_vga_client_pw_domain_dmc;
+		goto cleanup_flip;
 
 	init_llist_head(&i915->display.atomic_helper.free_list);
 	INIT_WORK(&i915->display.atomic_helper.free_work,
@@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 
 	return 0;
 
+cleanup_flip:
+	destroy_workqueue(i915->display.wq.flip);
+cleanup_modeset:
+	destroy_workqueue(i915->display.wq.modeset);
 cleanup_vga_client_pw_domain_dmc:
 	intel_dmc_ucode_fini(i915);
 	intel_power_domains_driver_remove(i915);
-- 
2.25.1

Re: [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Stanislaw Gruszka]

On Fri, Jan 06, 2023 at 05:09:34PM +0800, Jiasheng Jiang wrote:
> Add checks for the return value of alloc_workqueue and
> alloc_ordered_workqueue as they may return NULL pointer and cause NULL
> pointer dereference.
> Moreover, destroy them when fails later in order to avoid memory leak.
> Because in `drivers/gpu/drm/i915/i915_driver.c`, if
> intel_modeset_init_noirq fails, its workqueues will not be destroyed.
> 
> Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
> Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6c2686ecb62a..58f6840d6bd8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	intel_dmc_ucode_init(i915);
>  
>  	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
> +	if (!i915->display.wq.modeset) {
> +		ret = -ENOMEM;
> +		goto cleanup_vga_client_pw_domain_dmc;
> +	}
> +
>  	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
>  						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
> +	if (!i915->display.wq.flip) {
> +		ret = -ENOMEM;
> +		goto cleanup_modeset;
> +	}
>  
>  	intel_mode_config_init(i915);
>  
>  	ret = intel_cdclk_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_color_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_dbuf_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_bw_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	init_llist_head(&i915->display.atomic_helper.free_list);
>  	INIT_WORK(&i915->display.atomic_helper.free_work,
> @@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  
>  	return 0;
>  
> +cleanup_flip:
> +	destroy_workqueue(i915->display.wq.flip);
> +cleanup_modeset:
> +	destroy_workqueue(i915->display.wq.modeset);
>  cleanup_vga_client_pw_domain_dmc:
>  	intel_dmc_ucode_fini(i915);
>  	intel_power_domains_driver_remove(i915);
> -- 
> 2.25.1
> 

Re: [Intel-gfx] [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Stanislaw Gruszka]

On Fri, Jan 06, 2023 at 05:09:34PM +0800, Jiasheng Jiang wrote:
> Add checks for the return value of alloc_workqueue and
> alloc_ordered_workqueue as they may return NULL pointer and cause NULL
> pointer dereference.
> Moreover, destroy them when fails later in order to avoid memory leak.
> Because in `drivers/gpu/drm/i915/i915_driver.c`, if
> intel_modeset_init_noirq fails, its workqueues will not be destroyed.
> 
> Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
> Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6c2686ecb62a..58f6840d6bd8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	intel_dmc_ucode_init(i915);
>  
>  	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
> +	if (!i915->display.wq.modeset) {
> +		ret = -ENOMEM;
> +		goto cleanup_vga_client_pw_domain_dmc;
> +	}
> +
>  	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
>  						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
> +	if (!i915->display.wq.flip) {
> +		ret = -ENOMEM;
> +		goto cleanup_modeset;
> +	}
>  
>  	intel_mode_config_init(i915);
>  
>  	ret = intel_cdclk_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_color_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_dbuf_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_bw_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	init_llist_head(&i915->display.atomic_helper.free_list);
>  	INIT_WORK(&i915->display.atomic_helper.free_work,
> @@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  
>  	return 0;
>  
> +cleanup_flip:
> +	destroy_workqueue(i915->display.wq.flip);
> +cleanup_modeset:
> +	destroy_workqueue(i915->display.wq.modeset);
>  cleanup_vga_client_pw_domain_dmc:
>  	intel_dmc_ucode_fini(i915);
>  	intel_power_domains_driver_remove(i915);
> -- 
> 2.25.1
> 

Re: [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Stanislaw Gruszka]

On Fri, Jan 06, 2023 at 05:09:34PM +0800, Jiasheng Jiang wrote:
> Add checks for the return value of alloc_workqueue and
> alloc_ordered_workqueue as they may return NULL pointer and cause NULL
> pointer dereference.
> Moreover, destroy them when fails later in order to avoid memory leak.
> Because in `drivers/gpu/drm/i915/i915_driver.c`, if
> intel_modeset_init_noirq fails, its workqueues will not be destroyed.
> 
> Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
> Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6c2686ecb62a..58f6840d6bd8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	intel_dmc_ucode_init(i915);
>  
>  	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
> +	if (!i915->display.wq.modeset) {
> +		ret = -ENOMEM;
> +		goto cleanup_vga_client_pw_domain_dmc;
> +	}
> +
>  	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
>  						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
> +	if (!i915->display.wq.flip) {
> +		ret = -ENOMEM;
> +		goto cleanup_modeset;
> +	}
>  
>  	intel_mode_config_init(i915);
>  
>  	ret = intel_cdclk_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_color_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_dbuf_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_bw_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	init_llist_head(&i915->display.atomic_helper.free_list);
>  	INIT_WORK(&i915->display.atomic_helper.free_work,
> @@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  
>  	return 0;
>  
> +cleanup_flip:
> +	destroy_workqueue(i915->display.wq.flip);
> +cleanup_modeset:
> +	destroy_workqueue(i915->display.wq.modeset);
>  cleanup_vga_client_pw_domain_dmc:
>  	intel_dmc_ucode_fini(i915);
>  	intel_power_domains_driver_remove(i915);
> -- 
> 2.25.1
> 

Re: [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Daniel Vetter]

On Fri, Jan 06, 2023 at 05:09:34PM +0800, Jiasheng Jiang wrote:
> Add checks for the return value of alloc_workqueue and
> alloc_ordered_workqueue as they may return NULL pointer and cause NULL
> pointer dereference.
> Moreover, destroy them when fails later in order to avoid memory leak.
> Because in `drivers/gpu/drm/i915/i915_driver.c`, if
> intel_modeset_init_noirq fails, its workqueues will not be destroyed.
> 
> Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
> Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

So you have an entire pile of these, and I think that's a really good
excuse to
- create a drmm_alloc_workqueue helper for these (and
  drmm_alloc_ordered_workqueue) using the drmm_add_action_or_reset()
  function for automatic drm_device cleanup
- use that instead in all drivers, which means you do not have to make any
  error handling mor complex

Up for that? In that case please also convert any existing alloc*workqueue
callsites in drm, and make this all a patch series (since then there would
be dependencies).

Cheers, Daniel


> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6c2686ecb62a..58f6840d6bd8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	intel_dmc_ucode_init(i915);
>  
>  	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
> +	if (!i915->display.wq.modeset) {
> +		ret = -ENOMEM;
> +		goto cleanup_vga_client_pw_domain_dmc;
> +	}
> +
>  	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
>  						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
> +	if (!i915->display.wq.flip) {
> +		ret = -ENOMEM;
> +		goto cleanup_modeset;
> +	}
>  
>  	intel_mode_config_init(i915);
>  
>  	ret = intel_cdclk_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_color_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_dbuf_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_bw_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	init_llist_head(&i915->display.atomic_helper.free_list);
>  	INIT_WORK(&i915->display.atomic_helper.free_work,
> @@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  
>  	return 0;
>  
> +cleanup_flip:
> +	destroy_workqueue(i915->display.wq.flip);
> +cleanup_modeset:
> +	destroy_workqueue(i915->display.wq.modeset);
>  cleanup_vga_client_pw_domain_dmc:
>  	intel_dmc_ucode_fini(i915);
>  	intel_power_domains_driver_remove(i915);
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Daniel Vetter]

On Fri, Jan 06, 2023 at 05:09:34PM +0800, Jiasheng Jiang wrote:
> Add checks for the return value of alloc_workqueue and
> alloc_ordered_workqueue as they may return NULL pointer and cause NULL
> pointer dereference.
> Moreover, destroy them when fails later in order to avoid memory leak.
> Because in `drivers/gpu/drm/i915/i915_driver.c`, if
> intel_modeset_init_noirq fails, its workqueues will not be destroyed.
> 
> Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
> Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

So you have an entire pile of these, and I think that's a really good
excuse to
- create a drmm_alloc_workqueue helper for these (and
  drmm_alloc_ordered_workqueue) using the drmm_add_action_or_reset()
  function for automatic drm_device cleanup
- use that instead in all drivers, which means you do not have to make any
  error handling mor complex

Up for that? In that case please also convert any existing alloc*workqueue
callsites in drm, and make this all a patch series (since then there would
be dependencies).

Cheers, Daniel


> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6c2686ecb62a..58f6840d6bd8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	intel_dmc_ucode_init(i915);
>  
>  	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
> +	if (!i915->display.wq.modeset) {
> +		ret = -ENOMEM;
> +		goto cleanup_vga_client_pw_domain_dmc;
> +	}
> +
>  	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
>  						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
> +	if (!i915->display.wq.flip) {
> +		ret = -ENOMEM;
> +		goto cleanup_modeset;
> +	}
>  
>  	intel_mode_config_init(i915);
>  
>  	ret = intel_cdclk_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_color_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_dbuf_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_bw_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	init_llist_head(&i915->display.atomic_helper.free_list);
>  	INIT_WORK(&i915->display.atomic_helper.free_work,
> @@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  
>  	return 0;
>  
> +cleanup_flip:
> +	destroy_workqueue(i915->display.wq.flip);
> +cleanup_modeset:
> +	destroy_workqueue(i915->display.wq.modeset);
>  cleanup_vga_client_pw_domain_dmc:
>  	intel_dmc_ucode_fini(i915);
>  	intel_power_domains_driver_remove(i915);
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Daniel Vetter]

On Fri, Jan 06, 2023 at 05:09:34PM +0800, Jiasheng Jiang wrote:
> Add checks for the return value of alloc_workqueue and
> alloc_ordered_workqueue as they may return NULL pointer and cause NULL
> pointer dereference.
> Moreover, destroy them when fails later in order to avoid memory leak.
> Because in `drivers/gpu/drm/i915/i915_driver.c`, if
> intel_modeset_init_noirq fails, its workqueues will not be destroyed.
> 
> Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
> Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

So you have an entire pile of these, and I think that's a really good
excuse to
- create a drmm_alloc_workqueue helper for these (and
  drmm_alloc_ordered_workqueue) using the drmm_add_action_or_reset()
  function for automatic drm_device cleanup
- use that instead in all drivers, which means you do not have to make any
  error handling mor complex

Up for that? In that case please also convert any existing alloc*workqueue
callsites in drm, and make this all a patch series (since then there would
be dependencies).

Cheers, Daniel


> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6c2686ecb62a..58f6840d6bd8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8655,26 +8655,35 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	intel_dmc_ucode_init(i915);
>  
>  	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
> +	if (!i915->display.wq.modeset) {
> +		ret = -ENOMEM;
> +		goto cleanup_vga_client_pw_domain_dmc;
> +	}
> +
>  	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
>  						WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
> +	if (!i915->display.wq.flip) {
> +		ret = -ENOMEM;
> +		goto cleanup_modeset;
> +	}
>  
>  	intel_mode_config_init(i915);
>  
>  	ret = intel_cdclk_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_color_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_dbuf_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	ret = intel_bw_init(i915);
>  	if (ret)
> -		goto cleanup_vga_client_pw_domain_dmc;
> +		goto cleanup_flip;
>  
>  	init_llist_head(&i915->display.atomic_helper.free_list);
>  	INIT_WORK(&i915->display.atomic_helper.free_work,
> @@ -8686,6 +8695,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  
>  	return 0;
>  
> +cleanup_flip:
> +	destroy_workqueue(i915->display.wq.flip);
> +cleanup_modeset:
> +	destroy_workqueue(i915->display.wq.modeset);
>  cleanup_vga_client_pw_domain_dmc:
>  	intel_dmc_ucode_fini(i915);
>  	intel_power_domains_driver_remove(i915);
> -- 
> 2.25.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Add missing check and destroy for alloc_workqueue

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6334 bytes --]

== Series Details ==

Series: drm/i915: Add missing check and destroy for alloc_workqueue
URL   : https://patchwork.freedesktop.org/series/112623/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12569 -> Patchwork_112623v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/index.html

Participating hosts (41 -> 40)
------------------------------

  Additional (1): fi-rkl-11600 
  Missing    (2): fi-kbl-soraka fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_112623v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - fi-rkl-11600:       NOTRUN -> [SKIP][1] ([i915#7456])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@debugfs_test@basic-hwmon.html

  * igt@gem_huc_copy@huc-copy:
    - fi-rkl-11600:       NOTRUN -> [SKIP][2] ([i915#2190])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][3] ([i915#4613]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@gem_lmem_swapping@basic.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][4] ([i915#3282])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-rkl-11600:       NOTRUN -> [SKIP][5] ([i915#7561])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       NOTRUN -> [INCOMPLETE][6] ([i915#4817])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium_hpd@dp-hpd-fast:
    - fi-rkl-11600:       NOTRUN -> [SKIP][7] ([i915#7828]) +7 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@kms_chamelium_hpd@dp-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
    - fi-rkl-11600:       NOTRUN -> [SKIP][8] ([i915#4103])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][9] ([fdo#109285] / [i915#4098])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@primary_page_flip:
    - fi-rkl-11600:       NOTRUN -> [SKIP][10] ([i915#1072]) +3 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@kms_psr@primary_page_flip.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-rkl-11600:       NOTRUN -> [SKIP][11] ([i915#3555] / [i915#4098])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-read:
    - fi-rkl-11600:       NOTRUN -> [SKIP][12] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@prime_vgem@basic-read.html

  * igt@prime_vgem@basic-userptr:
    - fi-rkl-11600:       NOTRUN -> [SKIP][13] ([fdo#109295] / [i915#3301] / [i915#3708])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/fi-rkl-11600/igt@prime_vgem@basic-userptr.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_lrc:
    - {bat-adlp-9}:       [INCOMPLETE][14] ([i915#4983]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/bat-adlp-9/igt@i915_selftest@live@gt_lrc.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/bat-adlp-9/igt@i915_selftest@live@gt_lrc.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828


Build changes
-------------

  * Linux: CI_DRM_12569 -> Patchwork_112623v1

  CI-20190529: 20190529
  CI_DRM_12569: 739b2ee4e76d9cd64e5fbe834b682e550e496cf4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7115: c162d70b00c6f4cf6a0ba1ca7a7e2ad8f7190646 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_112623v1: 739b2ee4e76d9cd64e5fbe834b682e550e496cf4 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

ceed425fc416 drm/i915: Add missing check and destroy for alloc_workqueue

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/index.html

[-- Attachment #2: Type: text/html, Size: 7214 bytes --]

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Add missing check and destroy for alloc_workqueue

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 19169 bytes --]

== Series Details ==

Series: drm/i915: Add missing check and destroy for alloc_workqueue
URL   : https://patchwork.freedesktop.org/series/112623/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12569_full -> Patchwork_112623v1_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_112623v1_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_112623v1_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/index.html

Participating hosts (14 -> 10)
------------------------------

  Missing    (4): shard-rkl0 pig-kbl-iris pig-glk-j5005 pig-skl-6260u 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_112623v1_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-glk7/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk6/igt@gem_ppgtt@flink-and-close-vma-leak.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_addfb_basic@bad-pitch-256:
    - {shard-dg1}:        [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-dg1-14/igt@kms_addfb_basic@bad-pitch-256.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-dg1-19/igt@kms_addfb_basic@bad-pitch-256.html

  
Known issues
------------

  Here are the changes found in Patchwork_112623v1_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#2842])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-glk8/igt@gem_exec_fair@basic-none-share@rcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk7/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_lmem_swapping@massive:
    - shard-glk:          NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#4613])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk6/igt@gem_lmem_swapping@massive.html

  * igt@kms_ccs@pipe-b-bad-aux-stride-y_tiled_gen12_mc_ccs:
    - shard-glk:          NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#3886])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk6/igt@kms_ccs@pipe-b-bad-aux-stride-y_tiled_gen12_mc_ccs.html

  * igt@kms_frontbuffer_tracking@psr-rgb565-draw-mmap-gtt:
    - shard-glk:          NOTRUN -> [SKIP][9] ([fdo#109271]) +40 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk6/igt@kms_frontbuffer_tracking@psr-rgb565-draw-mmap-gtt.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area:
    - shard-glk:          NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#658])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk6/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html

  * igt@sysfs_clients@sema-25:
    - shard-glk:          NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#2994])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk6/igt@sysfs_clients@sema-25.html

  
#### Possible fixes ####

  * igt@fbdev@pan:
    - {shard-rkl}:        [SKIP][12] ([i915#2582]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-4/igt@fbdev@pan.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-6/igt@fbdev@pan.html

  * igt@feature_discovery@psr2:
    - {shard-rkl}:        [SKIP][14] ([i915#658]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-1/igt@feature_discovery@psr2.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-6/igt@feature_discovery@psr2.html

  * igt@gem_exec_balancer@fairslice:
    - {shard-rkl}:        [SKIP][16] ([i915#6259]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-5/igt@gem_exec_balancer@fairslice.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-1/igt@gem_exec_balancer@fairslice.html

  * igt@gem_exec_fair@basic-none@rcs0:
    - shard-glk:          [FAIL][18] ([i915#2842]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-glk8/igt@gem_exec_fair@basic-none@rcs0.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk5/igt@gem_exec_fair@basic-none@rcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - {shard-tglu}:       [FAIL][20] ([i915#2842]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-tglu-1/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-tglu-6/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_pread@snoop:
    - {shard-rkl}:        [SKIP][22] ([i915#3282]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-4/igt@gem_pread@snoop.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-5/igt@gem_pread@snoop.html

  * igt@i915_pipe_stress@stress-xrgb8888-untiled:
    - {shard-tglu}:       [SKIP][24] ([i915#1845]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-tglu-6/igt@i915_pipe_stress@stress-xrgb8888-untiled.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-tglu-1/igt@i915_pipe_stress@stress-xrgb8888-untiled.html

  * igt@i915_pm_rpm@cursor:
    - {shard-rkl}:        [SKIP][26] ([i915#1849]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-1/igt@i915_pm_rpm@cursor.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-6/igt@i915_pm_rpm@cursor.html

  * igt@i915_pm_rpm@pm-tiling:
    - {shard-rkl}:        [SKIP][28] ([fdo#109308]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-1/igt@i915_pm_rpm@pm-tiling.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-6/igt@i915_pm_rpm@pm-tiling.html

  * igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy:
    - {shard-tglu}:       [SKIP][30] ([i915#1845] / [i915#7651]) -> [PASS][31] +2 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-tglu-6/igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-tglu-1/igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size:
    - shard-glk:          [FAIL][32] ([i915#2346]) -> [PASS][33] +1 similar issue
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-glk8/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-gtt:
    - {shard-tglu}:       [SKIP][34] ([i915#1849]) -> [PASS][35] +2 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-tglu-6/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-gtt.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-tglu-1/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbc-tiling-linear:
    - {shard-rkl}:        [SKIP][36] ([i915#1849] / [i915#4098]) -> [PASS][37] +14 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-1/igt@kms_frontbuffer_tracking@fbc-tiling-linear.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-tiling-linear.html

  * igt@kms_psr@primary_render:
    - {shard-rkl}:        [SKIP][38] ([i915#1072]) -> [PASS][39] +2 similar issues
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-2/igt@kms_psr@primary_render.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-6/igt@kms_psr@primary_render.html

  * igt@kms_universal_plane@cursor-fb-leak-pipe-b:
    - {shard-rkl}:        [SKIP][40] ([i915#1845] / [i915#4070] / [i915#4098]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-2/igt@kms_universal_plane@cursor-fb-leak-pipe-b.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-6/igt@kms_universal_plane@cursor-fb-leak-pipe-b.html

  * igt@kms_vblank@pipe-b-ts-continuation-idle:
    - {shard-rkl}:        [SKIP][42] ([i915#1845] / [i915#4098]) -> [PASS][43] +19 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-1/igt@kms_vblank@pipe-b-ts-continuation-idle.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-6/igt@kms_vblank@pipe-b-ts-continuation-idle.html

  * igt@kms_vblank@pipe-d-ts-continuation-dpms-rpm:
    - {shard-tglu}:       [SKIP][44] ([i915#7651]) -> [PASS][45] +7 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-tglu-6/igt@kms_vblank@pipe-d-ts-continuation-dpms-rpm.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-tglu-1/igt@kms_vblank@pipe-d-ts-continuation-dpms-rpm.html

  * igt@prime_vgem@basic-fence-read:
    - {shard-rkl}:        [SKIP][46] ([fdo#109295] / [i915#3291] / [i915#3708]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12569/shard-rkl-4/igt@prime_vgem@basic-fence-read.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/shard-rkl-5/igt@prime_vgem@basic-fence-read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110542]: https://bugs.freedesktop.org/show_bug.cgi?id=110542
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#1902]: https://gitlab.freedesktop.org/drm/intel/issues/1902
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
  [i915#3528]: https://gitlab.freedesktop.org/drm/intel/issues/3528
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3547]: https://gitlab.freedesktop.org/drm/intel/issues/3547
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3558]: https://gitlab.freedesktop.org/drm/intel/issues/3558
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#426]: https://gitlab.freedesktop.org/drm/intel/issues/426
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4387]: https://gitlab.freedesktop.org/drm/intel/issues/4387
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#5327]: https://gitlab.freedesktop.org/drm/intel/issues/5327
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
  [i915#6259]: https://gitlab.freedesktop.org/drm/intel/issues/6259
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6334]: https://gitlab.freedesktop.org/drm/intel/issues/6334
  [i915#6344]: https://gitlab.freedesktop.org/drm/intel/issues/6344
  [i915#6355]: https://gitlab.freedesktop.org/drm/intel/issues/6355
  [i915#6412]: https://gitlab.freedesktop.org/drm/intel/issues/6412
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7128]: https://gitlab.freedesktop.org/drm/intel/issues/7128
  [i915#7294]: https://gitlab.freedesktop.org/drm/intel/issues/7294
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828


Build changes
-------------

  * Linux: CI_DRM_12569 -> Patchwork_112623v1
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_12569: 739b2ee4e76d9cd64e5fbe834b682e550e496cf4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7115: c162d70b00c6f4cf6a0ba1ca7a7e2ad8f7190646 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_112623v1: 739b2ee4e76d9cd64e5fbe834b682e550e496cf4 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112623v1/index.html

[-- Attachment #2: Type: text/html, Size: 14648 bytes --]

Re: [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Jiasheng Jiang]

On Sat, Jan 07, 2023 at 02:29:22AM +0800, Daniel Vetter wrote:
>On Fri, Jan 06, 2023 at 05:09:34PM +0800, Jiasheng Jiang wrote:
>> Add checks for the return value of alloc_workqueue and
>> alloc_ordered_workqueue as they may return NULL pointer and cause NULL
>> pointer dereference.
>> Moreover, destroy them when fails later in order to avoid memory leak.
>> Because in `drivers/gpu/drm/i915/i915_driver.c`, if
>> intel_modeset_init_noirq fails, its workqueues will not be destroyed.
>> 
>> Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
>> Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
>> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> 
> So you have an entire pile of these, and I think that's a really good
> excuse to
> - create a drmm_alloc_workqueue helper for these (and
>   drmm_alloc_ordered_workqueue) using the drmm_add_action_or_reset()
>   function for automatic drm_device cleanup
> - use that instead in all drivers, which means you do not have to make any
>   error handling mor complex
> 
> Up for that? In that case please also convert any existing alloc*workqueue
> callsites in drm, and make this all a patch series (since then there would
> be dependencies).

Fine, I have submitted two patches. The first one create the
drmm_alloc_workqueue and drmm_alloc_ordered_workqueue helpers. And the
second one replace the alloc*workqueue with DRM helpers in
`drivers/gpu/drm/i915/display/intel_display.c`.
If there is no problem in these two, I will submitted the other patches
that replace the alloc*workqueue in other DRM files.

Thanks,
Jiang

Re: [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue

[Jiasheng Jiang]

On Sat, Jan 07, 2023 at 02:29:22AM +0800, Daniel Vetter wrote:
>On Fri, Jan 06, 2023 at 05:09:34PM +0800, Jiasheng Jiang wrote:
>> Add checks for the return value of alloc_workqueue and
>> alloc_ordered_workqueue as they may return NULL pointer and cause NULL
>> pointer dereference.
>> Moreover, destroy them when fails later in order to avoid memory leak.
>> Because in `drivers/gpu/drm/i915/i915_driver.c`, if
>> intel_modeset_init_noirq fails, its workqueues will not be destroyed.
>> 
>> Fixes: c26a058680dc ("drm/i915: Use a high priority wq for nonblocking plane updates")
>> Fixes: 757fffcfdffb ("drm/i915: Put all non-blocking modesets onto an ordered wq")
>> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> 
> So you have an entire pile of these, and I think that's a really good
> excuse to
> - create a drmm_alloc_workqueue helper for these (and
>   drmm_alloc_ordered_workqueue) using the drmm_add_action_or_reset()
>   function for automatic drm_device cleanup
> - use that instead in all drivers, which means you do not have to make any
>   error handling mor complex
> 
> Up for that? In that case please also convert any existing alloc*workqueue
> callsites in drm, and make this all a patch series (since then there would
> be dependencies).

Fine, I have submitted two patches. The first one create the
drmm_alloc_workqueue and drmm_alloc_ordered_workqueue helpers. And the
second one replace the alloc*workqueue with DRM helpers in
`drivers/gpu/drm/i915/display/intel_display.c`.
If there is no problem in these two, I will submitted the other patches
that replace the alloc*workqueue in other DRM files.

Thanks,
Jiang