Thanks for preparing the patch, it looks like it will go another
round, but that happens to everyone.
To simplify to show a problem,
mutex_lock_interruptible(&dev->bus_mutex);
if (!sl->family_data)
return -ENODEV;
If family_data is NULL, then dev->bus_mutex is left locked. Your
earlier e-mails indicated that you weren't seeing any problems after
applying this patch, but I would have thought once this early return
was encountered, that bus would not be usable any longer because
bus_mutex would be locked. I don't know if the bus controller went
away, so there is effectively a new bus, because I assume you are
still able to read the temperature once this condition triggers?
It looks like you need to add mutex_unlock(&dev->bus_mutex); before
return -ENODEV; and that's how it is handled elsewhere in the
function, but you might want to put a printk before the return,
trigger it and verify it is getting hung up on the bus_mutex, it would
also be telling if you can remove the wire and the other w1 modules,
they should not let you remove them while a mutex is held.
The patch needs to be run through the checkpatch script, and it's
listing two issues, adding a space after the if (I also prefer not
having it, but this is for consistency), and missing Signed-off-by:
That would look like this, only with your information, it's a
tracing of where the changes came from and agreeing to the copyright,
you can read more in Documentation/SubmittingPatches if you haven't
already.
Signed-off-by: David Fries <da...@fries.net>
scripts/checkpatch.pl /tmp/w1_null_patch.patch
Thanks, keep up improving things.
On Fri, Feb 27, 2015 at 09:43:14AM +0100, Thorsten.Bschorr wrote:
> w1_slave_show unlocks the bus while waiting for the sensor
> to convert the temperature. If the sensor gets disconnected
> during that time, sl->family_data could get NULL.
> Note: the w1_slave pointer inside w1_slave_show is protected by
> sysfs-mutex
> ---
> drivers/w1/slaves/w1_therm.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index 1f11a20..3a14cb1 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -236,6 +236,12 @@ static ssize_t w1_slave_show(struct device *device,
> i = mutex_lock_interruptible(&dev->bus_mutex);
> if (i != 0)
> return i;
> +
> + /* Ensure that device is still there -
> + * it could have gone while we unlocked the bus.
> + * Note: sl is still protected by sysfs-mutex */
> + if(!sl->family_data)
> + return -ENODEV;
> } else if (!w1_strong_pullup) {
> sleep_rem = msleep_interruptible(tm);
> if (sleep_rem != 0) {
> --
> 1.8.4.5
>
>
> ---
> Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
> http://www.avast.com
>
--
David Fries <da...@fries.net> PGP pub CB1EE8F0
http://fries.net/~david/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
