* [PATCH] iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions
@ 2018-03-14 15:15 SF Markus Elfring
2018-03-17 19:54 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: SF Markus Elfring @ 2018-03-14 15:15 UTC (permalink / raw)
To: linux-iio, Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
Lars-Peter Clausen, Peter Meerwald-Stadler, Pravin Shedge,
Quentin Schulz
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Mar 2018 16:06:49 +0100
* Add jump targets so that a call of the function "mutex_unlock" is stored
only once in these function implementations.
* Replace 19 calls by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/iio/gyro/bmg160_core.c | 103 ++++++++++++++++++-----------------------
1 file changed, 45 insertions(+), 58 deletions(-)
diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
index 63ca31628a93..fa367fd7bc8c 100644
--- a/drivers/iio/gyro/bmg160_core.c
+++ b/drivers/iio/gyro/bmg160_core.c
@@ -499,21 +499,19 @@ static int bmg160_get_temp(struct bmg160_data *data, int *val)
mutex_lock(&data->mutex);
ret = bmg160_set_power_state(data, true);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
ret = regmap_read(data->regmap, BMG160_REG_TEMP, &raw_val);
if (ret < 0) {
dev_err(dev, "Error reading reg_temp\n");
bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ goto unlock;
}
*val = sign_extend32(raw_val, 7);
ret = bmg160_set_power_state(data, false);
+unlock:
mutex_unlock(&data->mutex);
if (ret < 0)
return ret;
@@ -529,22 +527,20 @@ static int bmg160_get_axis(struct bmg160_data *data, int axis, int *val)
mutex_lock(&data->mutex);
ret = bmg160_set_power_state(data, true);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
ret = regmap_bulk_read(data->regmap, BMG160_AXIS_TO_REG(axis), &raw_val,
sizeof(raw_val));
if (ret < 0) {
dev_err(dev, "Error reading axis %d\n", axis);
bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ goto unlock;
}
*val = sign_extend32(le16_to_cpu(raw_val), 15);
ret = bmg160_set_power_state(data, false);
+unlock:
mutex_unlock(&data->mutex);
if (ret < 0)
return ret;
@@ -632,19 +628,16 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
* mode to power on for other writes.
*/
ret = bmg160_set_power_state(data, true);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
+
ret = bmg160_set_bw(data, val);
if (ret < 0) {
bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ goto unlock;
}
- ret = bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+
+ goto set_power_state;
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
if (val2)
return -EINVAL;
@@ -653,18 +646,15 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
ret = bmg160_set_power_state(data, true);
if (ret < 0) {
bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ goto unlock;
}
ret = bmg160_set_filter(data, val);
if (ret < 0) {
bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ goto unlock;
}
- ret = bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+
+ goto set_power_state;
case IIO_CHAN_INFO_SCALE:
if (val)
return -EINVAL;
@@ -672,24 +662,27 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
mutex_lock(&data->mutex);
/* Refer to comments above for the suspend mode ops */
ret = bmg160_set_power_state(data, true);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
+
ret = bmg160_set_scale(data, val2);
if (ret < 0) {
bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ goto unlock;
}
- ret = bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+
+ goto set_power_state;
default:
return -EINVAL;
}
return -EINVAL;
+
+set_power_state:
+ ret = bmg160_set_power_state(data, false);
+unlock:
+ mutex_unlock(&data->mutex);
+ return ret;
}
static int bmg160_read_event(struct iio_dev *indio_dev,
@@ -763,8 +756,8 @@ static int bmg160_write_event_config(struct iio_dev *indio_dev,
if (!state && data->motion_trigger_on) {
data->ev_enable_state = 0;
- mutex_unlock(&data->mutex);
- return 0;
+ ret = 0;
+ goto unlock;
}
/*
* We will expect the enable and disable to do operation in
@@ -776,22 +769,19 @@ static int bmg160_write_event_config(struct iio_dev *indio_dev,
* is always on so sequence doesn't matter
*/
ret = bmg160_set_power_state(data, state);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
ret = bmg160_setup_any_motion_interrupt(data, state);
if (ret < 0) {
bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ goto unlock;
}
data->ev_enable_state = state;
+unlock:
mutex_unlock(&data->mutex);
-
- return 0;
+ return ret;
}
static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("100 200 400 1000 2000");
@@ -919,8 +909,8 @@ static int bmg160_data_rdy_trigger_set_state(struct iio_trigger *trig,
if (!state && data->ev_enable_state && data->motion_trigger_on) {
data->motion_trigger_on = false;
- mutex_unlock(&data->mutex);
- return 0;
+ ret = 0;
+ goto unlock;
}
/*
@@ -928,27 +918,24 @@ static int bmg160_data_rdy_trigger_set_state(struct iio_trigger *trig,
* enable/disable operation order
*/
ret = bmg160_set_power_state(data, state);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
+
if (data->motion_trig == trig)
ret = bmg160_setup_any_motion_interrupt(data, state);
else
ret = bmg160_setup_new_data_interrupt(data, state);
if (ret < 0) {
bmg160_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ goto unlock;
}
if (data->motion_trig == trig)
data->motion_trigger_on = state;
else
data->dready_trigger_on = state;
-
+unlock:
mutex_unlock(&data->mutex);
-
- return 0;
+ return ret;
}
static const struct iio_trigger_ops bmg160_trigger_ops = {
--
2.16.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions
2018-03-14 15:15 [PATCH] iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions SF Markus Elfring
@ 2018-03-17 19:54 ` Jonathan Cameron
2018-03-18 8:19 ` SF Markus Elfring
2018-03-18 10:05 ` [PATCH] " Greg Kroah-Hartman
0 siblings, 2 replies; 7+ messages in thread
From: Jonathan Cameron @ 2018-03-17 19:54 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-iio, Greg Kroah-Hartman, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Pravin Shedge,
Quentin Schulz, LKML, kernel-janitors
On Wed, 14 Mar 2018 16:15:32 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 14 Mar 2018 16:06:49 +0100
>
> * Add jump targets so that a call of the function "mutex_unlock" is stored
> only once in these function implementations.
>
> * Replace 19 calls by goto statements.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Hi Markus,
Some of these are good and sensible changes - others break the code.
Please be careful to fully check all the resulting paths and ensure
we don't change wether the lock is still held in all exit paths.
Note a function that isn't lockdep annotated should not be holding
any locks, that it took, upon exit.
> ---
> drivers/iio/gyro/bmg160_core.c | 103 ++++++++++++++++++-----------------------
> 1 file changed, 45 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
> index 63ca31628a93..fa367fd7bc8c 100644
> --- a/drivers/iio/gyro/bmg160_core.c
> +++ b/drivers/iio/gyro/bmg160_core.c
> @@ -499,21 +499,19 @@ static int bmg160_get_temp(struct bmg160_data *data, int *val)
>
> mutex_lock(&data->mutex);
> ret = bmg160_set_power_state(data, true);
> - if (ret < 0) {
> - mutex_unlock(&data->mutex);
> - return ret;
> - }
> + if (ret < 0)
> + goto unlock;
>
> ret = regmap_read(data->regmap, BMG160_REG_TEMP, &raw_val);
> if (ret < 0) {
> dev_err(dev, "Error reading reg_temp\n");
> bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> + goto unlock;
> }
>
> *val = sign_extend32(raw_val, 7);
> ret = bmg160_set_power_state(data, false);
> +unlock:
> mutex_unlock(&data->mutex);
> if (ret < 0)
> return ret;
> @@ -529,22 +527,20 @@ static int bmg160_get_axis(struct bmg160_data *data, int axis, int *val)
>
> mutex_lock(&data->mutex);
> ret = bmg160_set_power_state(data, true);
> - if (ret < 0) {
> - mutex_unlock(&data->mutex);
> - return ret;
> - }
> + if (ret < 0)
> + goto unlock;
>
> ret = regmap_bulk_read(data->regmap, BMG160_AXIS_TO_REG(axis), &raw_val,
> sizeof(raw_val));
> if (ret < 0) {
> dev_err(dev, "Error reading axis %d\n", axis);
> bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> + goto unlock;
> }
>
> *val = sign_extend32(le16_to_cpu(raw_val), 15);
> ret = bmg160_set_power_state(data, false);
> +unlock:
> mutex_unlock(&data->mutex);
> if (ret < 0)
> return ret;
> @@ -632,19 +628,16 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
> * mode to power on for other writes.
> */
> ret = bmg160_set_power_state(data, true);
> - if (ret < 0) {
> - mutex_unlock(&data->mutex);
> - return ret;
> - }
> + if (ret < 0)
> + goto unlock;
> +
> ret = bmg160_set_bw(data, val);
> if (ret < 0) {
> bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> + goto unlock;
> }
> - ret = bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> +
> + goto set_power_state;
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> if (val2)
> return -EINVAL;
> @@ -653,18 +646,15 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
> ret = bmg160_set_power_state(data, true);
> if (ret < 0) {
> bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> + goto unlock;
> }
> ret = bmg160_set_filter(data, val);
> if (ret < 0) {
> bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> + goto unlock;
> }
> - ret = bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> +
> + goto set_power_state;
> case IIO_CHAN_INFO_SCALE:
> if (val)
> return -EINVAL;
> @@ -672,24 +662,27 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
> mutex_lock(&data->mutex);
> /* Refer to comments above for the suspend mode ops */
> ret = bmg160_set_power_state(data, true);
> - if (ret < 0) {
> - mutex_unlock(&data->mutex);
> - return ret;
> - }
> + if (ret < 0)
> + goto unlock;
> +
> ret = bmg160_set_scale(data, val2);
> if (ret < 0) {
> bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> + goto unlock;
> }
> - ret = bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
Please keep the mutex_unlock in the same scope as the
mutex_lock.
I may make sense to take both outside the switch statement but
that needs careful consideration.
> - return ret;
> +
> + goto set_power_state;
> default:
> return -EINVAL;
We exit with the mutex locked now and it should not be.
> }
>
> return -EINVAL;
Mutex is still locked here and the return is wrong.
> +
> +set_power_state:
> + ret = bmg160_set_power_state(data, false);
> +unlock:
> + mutex_unlock(&data->mutex);
blank line before the return.
> + return ret;
> }
>
> static int bmg160_read_event(struct iio_dev *indio_dev,
> @@ -763,8 +756,8 @@ static int bmg160_write_event_config(struct iio_dev *indio_dev,
>
> if (!state && data->motion_trigger_on) {
> data->ev_enable_state = 0;
> - mutex_unlock(&data->mutex);
> - return 0;
> + ret = 0;
Put this in as the value at instantiation.
int ret = 0;
> + goto unlock;
> }
> /*
> * We will expect the enable and disable to do operation in
> @@ -776,22 +769,19 @@ static int bmg160_write_event_config(struct iio_dev *indio_dev,
> * is always on so sequence doesn't matter
> */
> ret = bmg160_set_power_state(data, state);
> - if (ret < 0) {
> - mutex_unlock(&data->mutex);
> - return ret;
> - }
> + if (ret < 0)
> + goto unlock;
>
> ret = bmg160_setup_any_motion_interrupt(data, state);
> if (ret < 0) {
> bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> + goto unlock;
> }
>
> data->ev_enable_state = state;
> +unlock:
> mutex_unlock(&data->mutex);
> -
Blank line preferred before the return ret.
> - return 0;
> + return ret;
> }
>
> static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("100 200 400 1000 2000");
> @@ -919,8 +909,8 @@ static int bmg160_data_rdy_trigger_set_state(struct iio_trigger *trig,
>
> if (!state && data->ev_enable_state && data->motion_trigger_on) {
> data->motion_trigger_on = false;
> - mutex_unlock(&data->mutex);
> - return 0;
> + ret = 0;
Setting ret where it is originally defined to 0 would be tidier.
int ret = 0;
> + goto unlock;
> }
>
> /*
> @@ -928,27 +918,24 @@ static int bmg160_data_rdy_trigger_set_state(struct iio_trigger *trig,
> * enable/disable operation order
> */
> ret = bmg160_set_power_state(data, state);
> - if (ret < 0) {
> - mutex_unlock(&data->mutex);
> - return ret;
> - }
> + if (ret < 0)
> + goto unlock;
> +
> if (data->motion_trig == trig)
> ret = bmg160_setup_any_motion_interrupt(data, state);
> else
> ret = bmg160_setup_new_data_interrupt(data, state);
> if (ret < 0) {
> bmg160_set_power_state(data, false);
> - mutex_unlock(&data->mutex);
> - return ret;
> + goto unlock;
> }
> if (data->motion_trig == trig)
> data->motion_trigger_on = state;
> else
> data->dready_trigger_on = state;
> -
> +unlock:
> mutex_unlock(&data->mutex);
> -
> - return 0;
I would prefer a blank line between the mutex_unlock and the return.
> + return ret;
> }
>
> static const struct iio_trigger_ops bmg160_trigger_ops = {
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions
2018-03-17 19:54 ` Jonathan Cameron
@ 2018-03-18 8:19 ` SF Markus Elfring
2018-03-18 10:15 ` Jonathan Cameron
2018-03-18 10:05 ` [PATCH] " Greg Kroah-Hartman
1 sibling, 1 reply; 7+ messages in thread
From: SF Markus Elfring @ 2018-03-18 8:19 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio
Cc: Greg Kroah-Hartman, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Pravin Shedge, Quentin Schulz, LKML,
kernel-janitors
Am 17.03.2018 um 20:54 schrieb Jonathan Cameron:
> On Wed, 14 Mar 2018 16:15:32 +0100
> SF Markus Elfring <elfring@users.sourceforge.net> wrote:
>
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Wed, 14 Mar 2018 16:06:49 +0100
>>
>> * Add jump targets so that a call of the function "mutex_unlock" is stored
>> only once in these function implementations.
>>
>> * Replace 19 calls by goto statements.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>
> Hi Markus,
>
> Some of these are good and sensible changes
Such feedback is nice.
> - others break the code.
Which concrete places do you find questionable here?
>> - return ret;
>> +
>> + goto set_power_state;
>> default:
>> return -EINVAL;
> We exit with the mutex locked now and it should not be.
I wonder about your source code interpretation here.
The mutex was (and is still only) locked within case branches, isn't it?
>
>> }
>>
>> return -EINVAL;
> Mutex is still locked here and the return is wrong.
Should this statement get any more software development attention?
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions
2018-03-18 8:19 ` SF Markus Elfring
@ 2018-03-18 10:15 ` Jonathan Cameron
2018-03-19 9:51 ` SF Markus Elfring
2018-03-24 9:08 ` SF Markus Elfring
0 siblings, 2 replies; 7+ messages in thread
From: Jonathan Cameron @ 2018-03-18 10:15 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-iio, Greg Kroah-Hartman, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Pravin Shedge,
Quentin Schulz, LKML, kernel-janitors
On Sun, 18 Mar 2018 09:19:47 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> Am 17.03.2018 um 20:54 schrieb Jonathan Cameron:
> > On Wed, 14 Mar 2018 16:15:32 +0100
> > SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> >
> >> From: Markus Elfring <elfring@users.sourceforge.net>
> >> Date: Wed, 14 Mar 2018 16:06:49 +0100
> >>
> >> * Add jump targets so that a call of the function "mutex_unlock" is stored
> >> only once in these function implementations.
> >>
> >> * Replace 19 calls by goto statements.
> >>
> >> This issue was detected by using the Coccinelle software.
> >>
> >> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> >
> > Hi Markus,
> >
> > Some of these are good and sensible changes
>
> Such feedback is nice.
>
>
> > - others break the code.
>
> Which concrete places do you find questionable here?
>
>
> >> - return ret;
> >> +
> >> + goto set_power_state;
> >> default:
> >> return -EINVAL;
> > We exit with the mutex locked now and it should not be.
>
> I wonder about your source code interpretation here.
> The mutex was (and is still only) locked within case branches, isn't it?
>
You are correct, this does however reflect the issue with the resulting
lack of balance here. I saw the mutex was getting unlocked outside
the local scope and so assumed that it was also take outside the local
scope. That isn't true, so we have hurt readability.
It might make sense to move the lock and unlock outside the switch statement
but we certainly don't want to the the confusion that the lack of balance is
causing here.
I read it quickly and got the wrong idea which generally implies it is not
as clear as we would like.
Hence this change isn't going anywhere I'm afraid.
Jonathan
>
> >
> >> }
> >>
> >> return -EINVAL;
> > Mutex is still locked here and the return is wrong.
>
> Should this statement get any more software development attention?
>
> Regards,
> Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions
2018-03-18 10:15 ` Jonathan Cameron
@ 2018-03-19 9:51 ` SF Markus Elfring
2018-03-24 9:08 ` SF Markus Elfring
1 sibling, 0 replies; 7+ messages in thread
From: SF Markus Elfring @ 2018-03-19 9:51 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio
Cc: Greg Kroah-Hartman, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Pravin Shedge, Quentin Schulz, LKML,
kernel-janitors
>> The mutex was (and is still only) locked within case branches, isn't it?
>>
> You are correct, this does however reflect the issue with the resulting
> lack of balance here.
I suggest to reconsider affected software aspects a bit more.
> I saw the mutex was getting unlocked outside the local scope and so assumed
> that it was also take outside the local scope.
Assumptions and corresponding expectations might need further clarifications.
> That isn't true, so we have hurt readability.
Does your conclusion need any adjustment?
> I read it quickly and got the wrong idea which generally implies it is not
> as clear as we would like.
>
> Hence this change isn't going anywhere I'm afraid.
I imagine that more time will be needed then to get used to additional adjustments
of implementation details in these functions.
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions
2018-03-18 10:15 ` Jonathan Cameron
2018-03-19 9:51 ` SF Markus Elfring
@ 2018-03-24 9:08 ` SF Markus Elfring
1 sibling, 0 replies; 7+ messages in thread
From: SF Markus Elfring @ 2018-03-24 9:08 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio
Cc: Greg Kroah-Hartman, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Pravin Shedge, Quentin Schulz, LKML,
kernel-janitors
>> The mutex was (and is still only) locked within case branches, isn't it?
>>
> You are correct, this does however reflect the issue with the resulting
> lack of balance here.
Do you find changes for the other function implementations easier to integrate?
Regards,
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions
2018-03-17 19:54 ` Jonathan Cameron
2018-03-18 8:19 ` SF Markus Elfring
@ 2018-03-18 10:05 ` Greg Kroah-Hartman
1 sibling, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-18 10:05 UTC (permalink / raw)
To: Jonathan Cameron
Cc: SF Markus Elfring, linux-iio, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Pravin Shedge, Quentin Schulz, LKML,
kernel-janitors
On Sat, Mar 17, 2018 at 07:54:22PM +0000, Jonathan Cameron wrote:
> On Wed, 14 Mar 2018 16:15:32 +0100
> SF Markus Elfring <elfring@users.sourceforge.net> wrote:
>
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Wed, 14 Mar 2018 16:06:49 +0100
> >
> > * Add jump targets so that a call of the function "mutex_unlock" is stored
> > only once in these function implementations.
> >
> > * Replace 19 calls by goto statements.
> >
> > This issue was detected by using the Coccinelle software.
> >
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>
> Hi Markus,
>
> Some of these are good and sensible changes - others break the code.
> Please be careful to fully check all the resulting paths and ensure
> we don't change wether the lock is still held in all exit paths.
> Note a function that isn't lockdep annotated should not be holding
> any locks, that it took, upon exit.
Please note that you are dealing with someone who is on many kernel
maintainer's email-blacklist, including my own. I've found it's just
not worth the time and energy in responding to these emails, it's a
time-sink you will never find your way out of.
good luck!
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-24 9:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 15:15 [PATCH] iio/gyro/bmg160_core: Improve unlocking of a mutex in five functions SF Markus Elfring
2018-03-17 19:54 ` Jonathan Cameron
2018-03-18 8:19 ` SF Markus Elfring
2018-03-18 10:15 ` Jonathan Cameron
2018-03-19 9:51 ` SF Markus Elfring
2018-03-24 9:08 ` SF Markus Elfring
2018-03-18 10:05 ` [PATCH] " Greg Kroah-Hartman
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).