From: Henrik Rydberg <rydberg@euromail.se> To: Guenter Roeck <linux@roeck-us.net> Cc: Josh Boyer <jwboyer@fedoraproject.org>, khali@linux-fr.org, lm-sensors@lm-sensors.org, "Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>, bugzilla@colorremedies.com Subject: Re: applesmc oops in 3.10/3.11 Date: Thu, 26 Sep 2013 08:34:53 +0200 [thread overview] Message-ID: <20130926063453.GA526@polaris.bitmath.org> (raw) In-Reply-To: <20130925220838.GB4184@roeck-us.net> > > > This suggests that initialization may be attempted more than once. The key cache > > > is allocated only once, but the number of keys is read for each attempt. > > > > > > No idea if that can happen, but if the number of keys can increase after > > > the first initialization attempt you would have an explanation for the crash. > > > > Good idea, and easy enough to test with the patch below. > > > Should we apply this patch even though it may not solve the specific problem ? Yes, why not - it certainly won't hurt. I am running it right now, so it is at least run-tested. > Again, not sure if the key count can change, but the current code is at the very > least inconsistent, as it keeps reading the key count without updating or > verifying the cache size. Yes - I agree that the error state is far-fetched, but it is hard to see any other logical explanation. There is of course always the possibility that the problem is somewhere else completely. Proper patch attached. Thanks, Henrik --- >From dedefba9167913c46e1896ce0624e68ffe95d532 Mon Sep 17 00:00:00 2001 From: Henrik Rydberg <rydberg@euromail.se> Date: Thu, 26 Sep 2013 08:33:16 +0200 Subject: [PATCH] hwmon: (applesmc) Check key count before proceeding After reports from Chris and Josh Boyer of a rare crash in applesmc, Guenter pointed at the initialization problem fixed below. The patch has not been verified to fix the crash, but should be applied regardless. Reported-by: <jwboyer@fedoraproject.org> Suggested-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Henrik Rydberg <rydberg@euromail.se> --- drivers/hwmon/applesmc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c index 62c2e32..98814d1 100644 --- a/drivers/hwmon/applesmc.c +++ b/drivers/hwmon/applesmc.c @@ -525,16 +525,25 @@ static int applesmc_init_smcreg_try(void) { struct applesmc_registers *s = &smcreg; bool left_light_sensor, right_light_sensor; + unsigned int count; u8 tmp[1]; int ret; if (s->init_complete) return 0; - ret = read_register_count(&s->key_count); + ret = read_register_count(&count); if (ret) return ret; + if (s->cache && s->key_count != count) { + pr_warn("key count changed from %d to %d\n", + s->key_count, count); + kfree(s->cache); + s->cache = NULL; + } + s->key_count = count; + if (!s->cache) s->cache = kcalloc(s->key_count, sizeof(*s->cache), GFP_KERNEL); if (!s->cache) -- 1.8.3.4
WARNING: multiple messages have this Message-ID (diff)
From: Henrik Rydberg <rydberg@euromail.se> To: Guenter Roeck <linux@roeck-us.net> Cc: Josh Boyer <jwboyer@fedoraproject.org>, khali@linux-fr.org, lm-sensors@lm-sensors.org, "Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>, bugzilla@colorremedies.com Subject: Re: [lm-sensors] applesmc oops in 3.10/3.11 Date: Thu, 26 Sep 2013 06:34:53 +0000 [thread overview] Message-ID: <20130926063453.GA526@polaris.bitmath.org> (raw) In-Reply-To: <20130925220838.GB4184@roeck-us.net> > > > This suggests that initialization may be attempted more than once. The key cache > > > is allocated only once, but the number of keys is read for each attempt. > > > > > > No idea if that can happen, but if the number of keys can increase after > > > the first initialization attempt you would have an explanation for the crash. > > > > Good idea, and easy enough to test with the patch below. > > > Should we apply this patch even though it may not solve the specific problem ? Yes, why not - it certainly won't hurt. I am running it right now, so it is at least run-tested. > Again, not sure if the key count can change, but the current code is at the very > least inconsistent, as it keeps reading the key count without updating or > verifying the cache size. Yes - I agree that the error state is far-fetched, but it is hard to see any other logical explanation. There is of course always the possibility that the problem is somewhere else completely. Proper patch attached. Thanks, Henrik --- From dedefba9167913c46e1896ce0624e68ffe95d532 Mon Sep 17 00:00:00 2001 From: Henrik Rydberg <rydberg@euromail.se> Date: Thu, 26 Sep 2013 08:33:16 +0200 Subject: [PATCH] hwmon: (applesmc) Check key count before proceeding After reports from Chris and Josh Boyer of a rare crash in applesmc, Guenter pointed at the initialization problem fixed below. The patch has not been verified to fix the crash, but should be applied regardless. Reported-by: <jwboyer@fedoraproject.org> Suggested-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Henrik Rydberg <rydberg@euromail.se> --- drivers/hwmon/applesmc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c index 62c2e32..98814d1 100644 --- a/drivers/hwmon/applesmc.c +++ b/drivers/hwmon/applesmc.c @@ -525,16 +525,25 @@ static int applesmc_init_smcreg_try(void) { struct applesmc_registers *s = &smcreg; bool left_light_sensor, right_light_sensor; + unsigned int count; u8 tmp[1]; int ret; if (s->init_complete) return 0; - ret = read_register_count(&s->key_count); + ret = read_register_count(&count); if (ret) return ret; + if (s->cache && s->key_count != count) { + pr_warn("key count changed from %d to %d\n", + s->key_count, count); + kfree(s->cache); + s->cache = NULL; + } + s->key_count = count; + if (!s->cache) s->cache = kcalloc(s->key_count, sizeof(*s->cache), GFP_KERNEL); if (!s->cache) -- 1.8.3.4 _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
next prev parent reply other threads:[~2013-09-26 6:33 UTC|newest] Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-09-25 19:06 applesmc oops in 3.10/3.11 Josh Boyer 2013-09-25 19:06 ` [lm-sensors] " Josh Boyer 2013-09-25 19:56 ` Guenter Roeck 2013-09-25 19:56 ` [lm-sensors] " Guenter Roeck 2013-09-25 21:48 ` Henrik Rydberg 2013-09-25 21:48 ` [lm-sensors] " Henrik Rydberg 2013-09-25 22:08 ` Guenter Roeck 2013-09-25 22:08 ` [lm-sensors] " Guenter Roeck 2013-09-26 6:34 ` Henrik Rydberg [this message] 2013-09-26 6:34 ` Henrik Rydberg 2013-09-26 10:36 ` Guenter Roeck 2013-09-26 10:36 ` [lm-sensors] " Guenter Roeck 2013-09-26 11:13 ` Henrik Rydberg 2013-09-26 11:13 ` [lm-sensors] " Henrik Rydberg 2013-09-26 10:53 ` Guenter Roeck 2013-09-26 10:53 ` [lm-sensors] " Guenter Roeck 2013-09-26 11:11 ` Henrik Rydberg 2013-09-26 11:11 ` [lm-sensors] " Henrik Rydberg 2013-09-27 16:21 ` Josh Boyer 2013-09-27 16:21 ` [lm-sensors] " Josh Boyer 2013-09-27 17:12 ` Guenter Roeck 2013-09-27 17:12 ` [lm-sensors] " Guenter Roeck 2013-09-27 17:41 ` Chris Murphy 2013-09-27 17:41 ` [lm-sensors] " Chris Murphy 2013-09-27 17:59 ` Guenter Roeck 2013-09-27 17:59 ` [lm-sensors] " Guenter Roeck 2013-09-27 18:03 ` Chris Murphy 2013-09-27 18:03 ` [lm-sensors] " Chris Murphy 2013-09-27 23:33 ` Guenter Roeck 2013-09-27 23:33 ` [lm-sensors] " Guenter Roeck 2013-10-01 1:57 ` Chris Murphy 2013-10-01 1:57 ` [lm-sensors] " Chris Murphy 2013-10-01 3:37 ` Guenter Roeck 2013-10-01 3:37 ` [lm-sensors] " Guenter Roeck 2013-10-01 10:55 ` Henrik Rydberg 2013-10-01 10:55 ` [lm-sensors] " Henrik Rydberg 2013-10-01 15:19 ` Guenter Roeck 2013-10-01 15:19 ` [lm-sensors] " Guenter Roeck 2013-10-01 15:33 ` Chris Murphy 2013-10-01 15:33 ` [lm-sensors] " Chris Murphy 2013-10-01 16:24 ` Guenter Roeck 2013-10-01 16:24 ` [lm-sensors] " Guenter Roeck 2013-10-02 1:09 ` Chris Murphy 2013-10-02 1:09 ` [lm-sensors] " Chris Murphy 2013-10-02 3:51 ` Guenter Roeck 2013-10-02 3:51 ` [lm-sensors] " Guenter Roeck 2013-10-02 3:55 ` Chris Murphy 2013-10-02 3:55 ` [lm-sensors] " Chris Murphy 2013-10-02 4:02 ` Guenter Roeck 2013-10-02 4:02 ` [lm-sensors] " Guenter Roeck 2013-10-02 9:53 ` Henrik Rydberg 2013-10-02 9:53 ` [lm-sensors] " Henrik Rydberg 2013-10-02 13:30 ` Guenter Roeck 2013-10-02 13:30 ` [lm-sensors] " Guenter Roeck 2013-10-02 16:34 ` Henrik Rydberg 2013-10-02 16:34 ` [lm-sensors] " Henrik Rydberg 2013-10-02 16:47 ` Guenter Roeck 2013-10-02 16:47 ` [lm-sensors] " Guenter Roeck 2013-10-02 17:24 ` Henrik Rydberg 2013-10-02 17:24 ` [lm-sensors] " Henrik Rydberg 2013-10-02 18:02 ` Guenter Roeck 2013-10-02 18:02 ` [lm-sensors] " Guenter Roeck 2013-10-02 18:33 ` Chris Murphy 2013-10-02 18:33 ` [lm-sensors] " Chris Murphy 2013-10-02 20:59 ` Guenter Roeck 2013-10-02 20:59 ` [lm-sensors] " Guenter Roeck 2013-10-02 21:34 ` Chris Murphy 2013-10-02 21:34 ` [lm-sensors] " Chris Murphy 2013-10-02 23:32 ` Guenter Roeck 2013-10-02 23:32 ` [lm-sensors] " Guenter Roeck 2013-10-07 23:42 ` Guenter Roeck 2013-10-07 23:42 ` [lm-sensors] " Guenter Roeck 2013-10-07 23:46 ` Chris Murphy 2013-10-07 23:46 ` [lm-sensors] " Chris Murphy 2013-10-08 15:48 ` Guenter Roeck 2013-10-08 15:48 ` [lm-sensors] " Guenter Roeck 2013-10-08 16:29 ` Henrik Rydberg 2013-10-08 16:29 ` [lm-sensors] " Henrik Rydberg 2013-10-08 16:29 ` Guenter Roeck 2013-10-08 16:29 ` [lm-sensors] " Guenter Roeck 2013-10-09 8:29 ` Henrik Rydberg 2013-10-09 8:29 ` [lm-sensors] " Henrik Rydberg 2013-10-09 16:52 ` Guenter Roeck 2013-10-09 16:52 ` [lm-sensors] " Guenter Roeck 2013-09-27 18:01 ` Guenter Roeck 2013-09-27 18:01 ` [lm-sensors] " Guenter Roeck
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=20130926063453.GA526@polaris.bitmath.org \ --to=rydberg@euromail.se \ --cc=bugzilla@colorremedies.com \ --cc=jwboyer@fedoraproject.org \ --cc=khali@linux-fr.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@roeck-us.net \ --cc=lm-sensors@lm-sensors.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.