From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 344F5C3DA7A for ; Fri, 6 Jan 2023 18:29:37 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 856D110E8B6; Fri, 6 Jan 2023 18:29:29 +0000 (UTC) Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6758310E8B4 for ; Fri, 6 Jan 2023 18:29:27 +0000 (UTC) Received: by mail-wr1-x436.google.com with SMTP id m7so2044377wrn.10 for ; Fri, 06 Jan 2023 10:29:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=heKjulj4vd8wqZaQwZ7f/NZciwr68hs+H70oCXrJOBw=; b=S1MWZ/D/HlFiipGQpawFD2vCb1GWHJWgR6DcDVxD3fbem8Q87eNJ6A26jBLGOAV8C/ lhgKAPXcSZSfKkxOxuex48uScuVL22MUT5mFenTkGZAfwyUD58TA8JSWseK+a2p24ybq sk/fletafETXHdG+AQUivRuZ/N4eSvSv9b/Zs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=heKjulj4vd8wqZaQwZ7f/NZciwr68hs+H70oCXrJOBw=; b=FZzTF3GB/gH7bxmgmi9G38QTgSpLKxXM6OHzKmiTZ4/Fiv6XEZ9TrH5Da3D/Ri9qSv 8Xsy0zBPy3ModyX/72mP/XJfEU6TDyblx2uknwJRk0y5YA4NGudrVjj0rSopG1DA8n3B nad8+yYzqStJ+XkE3iSHhJi4cgS5ExvXfyyOLwKPiSTkSF6gDdXGPUL2j44jZrHKnd+u sIo1xHnq9DfhV42boKvQVpmWOgp2E6FszN+w4ZCGowj7fzp5r2+fEq6NfaV9odhN/yfu N9sr+dkwZ/ehtiYupDARYyqEhqva/lk3z8aKIt4KwQnspisdxz4sY5mRi6sU/JYI6435 vUSg== X-Gm-Message-State: AFqh2kpLX8dhPTglSoZ9xZh/RaTUyFjvu2r0evJ0Q9wSGxF/y5vHgK00 kFcXvoFT0EoPYxuWvaQppA/+nQUJ/qIy4ZvK X-Google-Smtp-Source: AMrXdXsKFJLFLuub2PMJnENgYzeQIxxsh1DtQF692UW1umddMnScOHngnPOqtbxj45hf/NPGk+opHg== X-Received: by 2002:a5d:6a43:0:b0:26a:298:f067 with SMTP id t3-20020a5d6a43000000b0026a0298f067mr36459108wrw.43.1673029765955; Fri, 06 Jan 2023 10:29:25 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id c18-20020adffb52000000b0025e86026866sm2189989wrs.0.2023.01.06.10.29.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Jan 2023 10:29:24 -0800 (PST) Date: Fri, 6 Jan 2023 19:29:22 +0100 From: Daniel Vetter To: Jiasheng Jiang Message-ID: Mail-Followup-To: Jiasheng Jiang , jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, tvrtko.ursulin@linux.intel.com, airlied@gmail.com, ville.syrjala@linux.intel.com, manasi.d.navare@intel.com, stanislav.lisovskiy@intel.com, lucas.demarchi@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20230106090934.6348-1-jiasheng@iscas.ac.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230106090934.6348-1-jiasheng@iscas.ac.cn> X-Operating-System: Linux phenom 5.19.0-2-amd64 Subject: Re: [Intel-gfx] [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org, lucas.demarchi@intel.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, daniel@ffwll.ch, rodrigo.vivi@intel.com, airlied@gmail.com Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" 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 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 49470C3DA7A for ; Fri, 6 Jan 2023 18:29:31 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 102F410E8B4; Fri, 6 Jan 2023 18:29:29 +0000 (UTC) Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by gabe.freedesktop.org (Postfix) with ESMTPS id 67D6610E8B6 for ; Fri, 6 Jan 2023 18:29:27 +0000 (UTC) Received: by mail-wr1-x42c.google.com with SMTP id co23so2058380wrb.4 for ; Fri, 06 Jan 2023 10:29:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=heKjulj4vd8wqZaQwZ7f/NZciwr68hs+H70oCXrJOBw=; b=S1MWZ/D/HlFiipGQpawFD2vCb1GWHJWgR6DcDVxD3fbem8Q87eNJ6A26jBLGOAV8C/ lhgKAPXcSZSfKkxOxuex48uScuVL22MUT5mFenTkGZAfwyUD58TA8JSWseK+a2p24ybq sk/fletafETXHdG+AQUivRuZ/N4eSvSv9b/Zs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=heKjulj4vd8wqZaQwZ7f/NZciwr68hs+H70oCXrJOBw=; b=1vjBd5gcs11yfKmdEXz6qOrMBCXz549QkIdv5Na+2rwsl4yRNYyEkQeyB5bkG9stYH Fwtz6wGbdf/+1JR7UTbokO4kRaaMMcSNO7X4mX5/CQP885Nv5kXm7a8eF881BEJIdN0p OvsWALKz9d/M+BdtetejXxU1SiDdbSimJ+fDp+U0WI8OptnmlWOCdeAoXNTowgb6uhqk gn0PccxHdO/zFPtIs3dzmr7btXz1pT51pV/y0Pq7OhCMYqcJLLYU/fI8y7dIl2lr7vOt ShC3s2+qQzxXHNgmCO5IyiWY8MsVnKnTpIbuwi4NQPqEj78XJwOVSMYKJLkoER6Ult3q m6Rw== X-Gm-Message-State: AFqh2kr/eF5iP88vSvcuuKuIGqa7sBumjCODwnJDtT3H/VmWTceJhsvl FARmS5xkAVp6s+Au5PrKGeGT8A== X-Google-Smtp-Source: AMrXdXsKFJLFLuub2PMJnENgYzeQIxxsh1DtQF692UW1umddMnScOHngnPOqtbxj45hf/NPGk+opHg== X-Received: by 2002:a5d:6a43:0:b0:26a:298:f067 with SMTP id t3-20020a5d6a43000000b0026a0298f067mr36459108wrw.43.1673029765955; Fri, 06 Jan 2023 10:29:25 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id c18-20020adffb52000000b0025e86026866sm2189989wrs.0.2023.01.06.10.29.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Jan 2023 10:29:24 -0800 (PST) Date: Fri, 6 Jan 2023 19:29:22 +0100 From: Daniel Vetter To: Jiasheng Jiang Subject: Re: [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue Message-ID: Mail-Followup-To: Jiasheng Jiang , jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, tvrtko.ursulin@linux.intel.com, airlied@gmail.com, ville.syrjala@linux.intel.com, manasi.d.navare@intel.com, stanislav.lisovskiy@intel.com, lucas.demarchi@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20230106090934.6348-1-jiasheng@iscas.ac.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230106090934.6348-1-jiasheng@iscas.ac.cn> X-Operating-System: Linux phenom 5.19.0-2-amd64 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: tvrtko.ursulin@linux.intel.com, stanislav.lisovskiy@intel.com, intel-gfx@lists.freedesktop.org, lucas.demarchi@intel.com, linux-kernel@vger.kernel.org, manasi.d.navare@intel.com, dri-devel@lists.freedesktop.org, rodrigo.vivi@intel.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" 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 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F0E13C54EBD for ; Fri, 6 Jan 2023 18:29:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235816AbjAFS3a (ORCPT ); Fri, 6 Jan 2023 13:29:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58904 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231367AbjAFS32 (ORCPT ); Fri, 6 Jan 2023 13:29:28 -0500 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5C15962E3 for ; Fri, 6 Jan 2023 10:29:27 -0800 (PST) Received: by mail-wr1-x42f.google.com with SMTP id w1so2047866wrt.8 for ; Fri, 06 Jan 2023 10:29:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=heKjulj4vd8wqZaQwZ7f/NZciwr68hs+H70oCXrJOBw=; b=S1MWZ/D/HlFiipGQpawFD2vCb1GWHJWgR6DcDVxD3fbem8Q87eNJ6A26jBLGOAV8C/ lhgKAPXcSZSfKkxOxuex48uScuVL22MUT5mFenTkGZAfwyUD58TA8JSWseK+a2p24ybq sk/fletafETXHdG+AQUivRuZ/N4eSvSv9b/Zs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=heKjulj4vd8wqZaQwZ7f/NZciwr68hs+H70oCXrJOBw=; b=QLbbDUrR1xAA3e/dBlwjTsQh+wBh516GIKWbNNOIu4npgtbEF5iyqNnaPr9cdkx6Kf 9ZEs4kWuL+g25noDzkn5IJ20NTFLEDf4YtUM7uI1EbJDeQAueMAn2d+hKlk07DUHgODm 1lLw6x/HcMsu1t6YN01ZH6mNAnVojUfYlCwotcRpYHpyJvqnGhvZJ0EED7yZR+n7N+rY uU8ksDhNjnbeXFtu9pHOiigGgnGGMnbF/qDiLsgzYezPfhJVMEvGY6TjzAJ4czeomWWp QAtM93oLkAiYOZQgZ9XbbRghjKmr+u6wrvd7x++w8OPr4lBD8IzeRj0iq7H70PRAWbWL y9rg== X-Gm-Message-State: AFqh2kocb6xU5W13oUSYhC1+W/dxmtj0Jlzz6ThTMuLLhEDlhSqsW50j lnVorScJpcWD2I1rAFv4n0ncOw== X-Google-Smtp-Source: AMrXdXsKFJLFLuub2PMJnENgYzeQIxxsh1DtQF692UW1umddMnScOHngnPOqtbxj45hf/NPGk+opHg== X-Received: by 2002:a5d:6a43:0:b0:26a:298:f067 with SMTP id t3-20020a5d6a43000000b0026a0298f067mr36459108wrw.43.1673029765955; Fri, 06 Jan 2023 10:29:25 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id c18-20020adffb52000000b0025e86026866sm2189989wrs.0.2023.01.06.10.29.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Jan 2023 10:29:24 -0800 (PST) Date: Fri, 6 Jan 2023 19:29:22 +0100 From: Daniel Vetter To: Jiasheng Jiang Cc: jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, tvrtko.ursulin@linux.intel.com, airlied@gmail.com, daniel@ffwll.ch, ville.syrjala@linux.intel.com, manasi.d.navare@intel.com, stanislav.lisovskiy@intel.com, lucas.demarchi@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/i915: Add missing check and destroy for alloc_workqueue Message-ID: Mail-Followup-To: Jiasheng Jiang , jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, tvrtko.ursulin@linux.intel.com, airlied@gmail.com, ville.syrjala@linux.intel.com, manasi.d.navare@intel.com, stanislav.lisovskiy@intel.com, lucas.demarchi@intel.com, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20230106090934.6348-1-jiasheng@iscas.ac.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230106090934.6348-1-jiasheng@iscas.ac.cn> X-Operating-System: Linux phenom 5.19.0-2-amd64 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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