From: Hans de Goede <hdegoede@redhat.com>
To: Armin Wolf <W_Armin@gmx.de>, markgross@kernel.org
Cc: jdelvare@suse.com, linux@roeck-us.net,
platform-driver-x86@vger.kernel.org, linux-hwmon@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] platform/x86: dell-ddv: Fix cache invalidation on resume
Date: Wed, 1 Mar 2023 13:15:04 +0100 [thread overview]
Message-ID: <15d7b77d-c66e-6e31-e930-7595cdabcd11@redhat.com> (raw)
In-Reply-To: <20230218115318.20662-1-W_Armin@gmx.de>
Hi,
On 2/18/23 12:53, Armin Wolf wrote:
> If one or both sensor buffers could not be initialized, either
> due to missing hardware support or due to some error during probing,
> the resume handler will encounter undefined behaviour when
> attempting to lock buffers then protected by an uninitialized or
> destroyed mutex.
> Fix this by introducing a "active" flag which is set during probe,
> and only invalidate buffers which where flaged as "active".
>
> Tested on a Dell Inspiron 3505.
>
> Fixes: 3b7eeff93d29 ("platform/x86: dell-ddv: Add hwmon support")
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
> Changes in v2:
> - move checking of the "active" flag inside
> dell_wmi_ddv_hwmon_cache_invalidate()
Thanks, I've applied this patch to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
I'll rebase that branch once 6.3-rc1 is out and then push the rebased
patch to the fixes branch and include it in my next 6.3 fixes pull-req
to Linus.
Regards,
Hans
> ---
> drivers/platform/x86/dell/dell-wmi-ddv.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
> index d547c9d09725..eff4e9649faf 100644
> --- a/drivers/platform/x86/dell/dell-wmi-ddv.c
> +++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
> @@ -96,6 +96,7 @@ struct combined_chip_info {
> };
>
> struct dell_wmi_ddv_sensors {
> + bool active;
> struct mutex lock; /* protect caching */
> unsigned long timestamp;
> union acpi_object *obj;
> @@ -520,6 +521,9 @@ static struct hwmon_channel_info *dell_wmi_ddv_channel_create(struct device *dev
>
> static void dell_wmi_ddv_hwmon_cache_invalidate(struct dell_wmi_ddv_sensors *sensors)
> {
> + if (!sensors->active)
> + return;
> +
> mutex_lock(&sensors->lock);
> kfree(sensors->obj);
> sensors->obj = NULL;
> @@ -530,6 +534,7 @@ static void dell_wmi_ddv_hwmon_cache_destroy(void *data)
> {
> struct dell_wmi_ddv_sensors *sensors = data;
>
> + sensors->active = false;
> mutex_destroy(&sensors->lock);
> kfree(sensors->obj);
> }
> @@ -549,6 +554,7 @@ static struct hwmon_channel_info *dell_wmi_ddv_channel_init(struct wmi_device *w
> return ERR_PTR(ret);
>
> mutex_init(&sensors->lock);
> + sensors->active = true;
>
> ret = devm_add_action_or_reset(&wdev->dev, dell_wmi_ddv_hwmon_cache_destroy, sensors);
> if (ret < 0)
> @@ -852,7 +858,7 @@ static int dell_wmi_ddv_resume(struct device *dev)
> {
> struct dell_wmi_ddv_data *data = dev_get_drvdata(dev);
>
> - /* Force re-reading of all sensors */
> + /* Force re-reading of all active sensors */
> dell_wmi_ddv_hwmon_cache_invalidate(&data->fans);
> dell_wmi_ddv_hwmon_cache_invalidate(&data->temps);
>
> --
> 2.30.2
>
prev parent reply other threads:[~2023-03-01 12:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-18 11:53 [PATCH v2 1/2] platform/x86: dell-ddv: Fix cache invalidation on resume Armin Wolf
2023-02-18 11:53 ` [PATCH v2 2/2] platform/x86: dell-ddv: Fix temperature scaling Armin Wolf
2023-03-01 12:19 ` Hans de Goede
2023-02-28 13:21 ` [PATCH v2 1/2] platform/x86: dell-ddv: Fix cache invalidation on resume Armin Wolf
2023-03-01 12:15 ` Hans de Goede [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=15d7b77d-c66e-6e31-e930-7595cdabcd11@redhat.com \
--to=hdegoede@redhat.com \
--cc=W_Armin@gmx.de \
--cc=jdelvare@suse.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=markgross@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).