* [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"
[not found] ` <59e51c7d-f7bb-c55f-d2fe-06dccfef6a99@users.sourceforge.net>
@ 2016-07-16 20:22 ` SF Markus Elfring
2016-07-16 20:23 ` [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection SF Markus Elfring
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-16 20:22 UTC (permalink / raw)
To: xen-devel, linux-scsi, Jürgen Groß,
David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 21:21:05 +0200
The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/xen/xen-scsiback.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index d6950e0..4a48c06 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -627,8 +627,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
transport_generic_free_cmd(&pending_req->se_cmd, 1);
return;
err:
- if (tmr)
- kfree(tmr);
+ kfree(tmr);
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
}
--
2.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
[not found] ` <59e51c7d-f7bb-c55f-d2fe-06dccfef6a99@users.sourceforge.net>
2016-07-16 20:22 ` [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree" SF Markus Elfring
@ 2016-07-16 20:23 ` SF Markus Elfring
2016-07-16 20:24 ` [PATCH 3/3] xen-scsiback: Pass a failure indication as a constant SF Markus Elfring
` (3 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-16 20:23 UTC (permalink / raw)
To: xen-devel, linux-scsi, Jürgen Groß,
David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 21:42:42 +0200
The kfree() function was called in one case by the
scsiback_device_action() function during error handling
even if the passed variable "tmr" contained a null pointer.
Adjust jump targets according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/xen/xen-scsiback.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 4a48c06..7612bc9 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
if (!tmr) {
target_put_sess_cmd(se_cmd);
- goto err;
+ goto do_resp;
}
init_waitqueue_head(&tmr->tmr_wait);
@@ -616,7 +616,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
unpacked_lun, tmr, act, GFP_KERNEL,
tag, TARGET_SCF_ACK_KREF);
if (rc)
- goto err;
+ goto free_tmr;
wait_event(tmr->tmr_wait, atomic_read(&tmr->tmr_complete));
@@ -626,8 +626,9 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
transport_generic_free_cmd(&pending_req->se_cmd, 1);
return;
-err:
+free_tmr:
kfree(tmr);
+do_resp:
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
}
--
2.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] xen-scsiback: Pass a failure indication as a constant
[not found] ` <59e51c7d-f7bb-c55f-d2fe-06dccfef6a99@users.sourceforge.net>
2016-07-16 20:22 ` [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree" SF Markus Elfring
2016-07-16 20:23 ` [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection SF Markus Elfring
@ 2016-07-16 20:24 ` SF Markus Elfring
[not found] ` <fa839c03-b076-0264-c363-5581d0298632@users.sourceforge.net>
` (2 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-16 20:24 UTC (permalink / raw)
To: xen-devel, linux-scsi, Jürgen Groß,
David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 21:55:01 +0200
Pass the constant "FAILED" in a function call directly instead of
using an intialisation for a local variable.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/xen/xen-scsiback.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 7612bc9..ec5546c 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -601,7 +601,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
struct se_cmd *se_cmd = &pending_req->se_cmd;
struct scsiback_tmr *tmr;
u64 unpacked_lun = pending_req->v2p->lun;
- int rc, err = FAILED;
+ int rc, err;
tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
if (!tmr) {
@@ -629,7 +629,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
free_tmr:
kfree(tmr);
do_resp:
- scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
+ scsiback_do_resp_with_sense(NULL, FAILED, 0, pending_req);
}
/*
--
2.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
[parent not found: <fa839c03-b076-0264-c363-5581d0298632@users.sourceforge.net>]
* Re: [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"
[not found] ` <fa839c03-b076-0264-c363-5581d0298632@users.sourceforge.net>
@ 2016-07-18 5:01 ` Juergen Gross
0 siblings, 0 replies; 20+ messages in thread
From: Juergen Gross @ 2016-07-18 5:01 UTC (permalink / raw)
To: SF Markus Elfring, xen-devel, linux-scsi, David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
On 16/07/16 22:22, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 16 Jul 2016 21:21:05 +0200
>
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <e447cf62-ffc7-4b82-cff2-947c8874756a@users.sourceforge.net>]
[parent not found: <87e55a72-71ad-d0a1-b936-12565bbc6508@users.sourceforge.net>]
* Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
[not found] ` <87e55a72-71ad-d0a1-b936-12565bbc6508@users.sourceforge.net>
@ 2016-07-18 5:11 ` Juergen Gross
[not found] ` <fb5824ad-fb09-e471-7dc4-3e32564cc334@suse.com>
1 sibling, 0 replies; 20+ messages in thread
From: Juergen Gross @ 2016-07-18 5:11 UTC (permalink / raw)
To: SF Markus Elfring, xen-devel, linux-scsi, David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
On 16/07/16 22:23, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 16 Jul 2016 21:42:42 +0200
>
> The kfree() function was called in one case by the
> scsiback_device_action() function during error handling
> even if the passed variable "tmr" contained a null pointer.
>
> Adjust jump targets according to the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/xen/xen-scsiback.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
> index 4a48c06..7612bc9 100644
> --- a/drivers/xen/xen-scsiback.c
> +++ b/drivers/xen/xen-scsiback.c
> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
> tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
> if (!tmr) {
> target_put_sess_cmd(se_cmd);
> - goto err;
> + goto do_resp;
> }
Hmm, I'm not convinced this is an improvement.
I'd rather rename the new error label to "put_cmd" and get rid of the
braces in above if statement:
- if (!tmr) {
- target_put_sess_cmd(se_cmd);
- goto err;
- }
+ if (!tmr)
+ goto put_cmd;
and then in the error path:
-err:
+put_cmd:
+ target_put_sess_cmd(se_cmd);
+free_tmr:
kfree(tmr);
Juergen
>
> init_waitqueue_head(&tmr->tmr_wait);
> @@ -616,7 +616,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
> unpacked_lun, tmr, act, GFP_KERNEL,
> tag, TARGET_SCF_ACK_KREF);
> if (rc)
> - goto err;
> + goto free_tmr;
>
> wait_event(tmr->tmr_wait, atomic_read(&tmr->tmr_complete));
>
> @@ -626,8 +626,9 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
> scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
> transport_generic_free_cmd(&pending_req->se_cmd, 1);
> return;
> -err:
> +free_tmr:
> kfree(tmr);
> +do_resp:
> scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
> }
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <fb5824ad-fb09-e471-7dc4-3e32564cc334@suse.com>]
* Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
[not found] ` <fb5824ad-fb09-e471-7dc4-3e32564cc334@suse.com>
@ 2016-07-19 14:56 ` SF Markus Elfring
2016-07-20 4:35 ` Juergen Gross
[not found] ` <83b0b1e2-900b-eefc-160b-7d837730571c@suse.com>
0 siblings, 2 replies; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-19 14:56 UTC (permalink / raw)
To: Jürgen Groß
Cc: linux-scsi, kernel-janitors, LKML, Julia Lawall, David Vrabel,
xen-devel, Boris Ostrovsky
>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>> tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>> if (!tmr) {
>> target_put_sess_cmd(se_cmd);
>> - goto err;
>> + goto do_resp;
>> }
>
> Hmm, I'm not convinced this is an improvement.
>
> I'd rather rename the new error label to "put_cmd" and get rid of the
> braces in above if statement:
>
> - if (!tmr) {
> - target_put_sess_cmd(se_cmd);
> - goto err;
> - }
> + if (!tmr)
> + goto put_cmd;
>
> and then in the error path:
>
> -err:
> +put_cmd:
> + target_put_sess_cmd(se_cmd);
I am unsure on the relevance of this function on such a source position.
Would it make sense to move it further down at the end?
> +free_tmr:
> kfree(tmr);
How do you think about to skip this function call after a memory
allocation failure?
Regards,
Markus
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
2016-07-19 14:56 ` SF Markus Elfring
@ 2016-07-20 4:35 ` Juergen Gross
[not found] ` <83b0b1e2-900b-eefc-160b-7d837730571c@suse.com>
1 sibling, 0 replies; 20+ messages in thread
From: Juergen Gross @ 2016-07-20 4:35 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-scsi, kernel-janitors, LKML, Julia Lawall, David Vrabel,
xen-devel, Boris Ostrovsky
On 19/07/16 16:56, SF Markus Elfring wrote:
>>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>>> tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>> if (!tmr) {
>>> target_put_sess_cmd(se_cmd);
>>> - goto err;
>>> + goto do_resp;
>>> }
>>
>> Hmm, I'm not convinced this is an improvement.
>>
>> I'd rather rename the new error label to "put_cmd" and get rid of the
>> braces in above if statement:
>>
>> - if (!tmr) {
>> - target_put_sess_cmd(se_cmd);
>> - goto err;
>> - }
>> + if (!tmr)
>> + goto put_cmd;
>>
>> and then in the error path:
>>
>> -err:
>> +put_cmd:
>> + target_put_sess_cmd(se_cmd);
>
> I am unsure on the relevance of this function on such a source position.
> Would it make sense to move it further down at the end?
You only want to call it in the first error case (allocation failure).
>> +free_tmr:
>> kfree(tmr);
>
> How do you think about to skip this function call after a memory
> allocation failure?
I think this just doesn't matter. If it were a hot path, yes. But trying
to do micro-optimizations in an error path is just not worth the effort.
I like a linear error path containing all the needed cleanups best.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <83b0b1e2-900b-eefc-160b-7d837730571c@suse.com>]
* Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
[not found] ` <83b0b1e2-900b-eefc-160b-7d837730571c@suse.com>
@ 2016-07-20 5:10 ` SF Markus Elfring
2016-07-20 5:26 ` Juergen Gross
[not found] ` <85b948e0-bf37-dff0-6792-77b5dacef9a0@suse.com>
0 siblings, 2 replies; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-20 5:10 UTC (permalink / raw)
To: Jürgen Groß
Cc: linux-scsi, kernel-janitors, LKML, Julia Lawall, David Vrabel,
xen-devel, Boris Ostrovsky
>>>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>>>> tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>>> if (!tmr) {
>>>> target_put_sess_cmd(se_cmd);
>>>> - goto err;
>>>> + goto do_resp;
>>>> }
>>>
>>> Hmm, I'm not convinced this is an improvement.
>>>
>>> I'd rather rename the new error label to "put_cmd" and get rid of the
>>> braces in above if statement:
>>>
>>> - if (!tmr) {
>>> - target_put_sess_cmd(se_cmd);
>>> - goto err;
>>> - }
>>> + if (!tmr)
>>> + goto put_cmd;
>>>
>>> and then in the error path:
>>>
>>> -err:
>>> +put_cmd:
>>> + target_put_sess_cmd(se_cmd);
>>
>> I am unsure on the relevance of this function on such a source position.
>> Would it make sense to move it further down at the end?
>
> You only want to call it in the first error case (allocation failure).
Thanks for your clarification.
I find that my update suggestion (from Saturday) is still appropriate
in this case.
https://lkml.org/lkml/2016/7/16/172
>>> +free_tmr:
>>> kfree(tmr);
>>
>> How do you think about to skip this function call after a memory
>> allocation failure?
>
> I think this just doesn't matter. If it were a hot path, yes. But trying
> to do micro-optimizations in an error path is just not worth the effort.
Would you like to reduce also the amount of function calls in such special
run-time situations?
> I like a linear error path containing all the needed cleanups best.
I would prefer to keep the discussed single function call within
the basic block of the if statement.
Have we got different opinions about the shown implementation details?
Regards,
Markus
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
2016-07-20 5:10 ` SF Markus Elfring
@ 2016-07-20 5:26 ` Juergen Gross
[not found] ` <85b948e0-bf37-dff0-6792-77b5dacef9a0@suse.com>
1 sibling, 0 replies; 20+ messages in thread
From: Juergen Gross @ 2016-07-20 5:26 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-scsi, kernel-janitors, LKML, Julia Lawall, David Vrabel,
xen-devel, Boris Ostrovsky
On 20/07/16 07:10, SF Markus Elfring wrote:
>>>>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
>>>>> tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>>>> if (!tmr) {
>>>>> target_put_sess_cmd(se_cmd);
>>>>> - goto err;
>>>>> + goto do_resp;
>>>>> }
>>>>
>>>> Hmm, I'm not convinced this is an improvement.
>>>>
>>>> I'd rather rename the new error label to "put_cmd" and get rid of the
>>>> braces in above if statement:
>>>>
>>>> - if (!tmr) {
>>>> - target_put_sess_cmd(se_cmd);
>>>> - goto err;
>>>> - }
>>>> + if (!tmr)
>>>> + goto put_cmd;
>>>>
>>>> and then in the error path:
>>>>
>>>> -err:
>>>> +put_cmd:
>>>> + target_put_sess_cmd(se_cmd);
>>>
>>> I am unsure on the relevance of this function on such a source position.
>>> Would it make sense to move it further down at the end?
>>
>> You only want to call it in the first error case (allocation failure).
>
> Thanks for your clarification.
>
> I find that my update suggestion (from Saturday) is still appropriate
> in this case.
> https://lkml.org/lkml/2016/7/16/172
And I still think it isn't an improvement: Nack
>>>> +free_tmr:
>>>> kfree(tmr);
>>>
>>> How do you think about to skip this function call after a memory
>>> allocation failure?
>>
>> I think this just doesn't matter. If it were a hot path, yes. But trying
>> to do micro-optimizations in an error path is just not worth the effort.
>
> Would you like to reduce also the amount of function calls in such special
> run-time situations?
I just don't care for the extra 2 or 3 nsecs. Readability is more
important here.
>> I like a linear error path containing all the needed cleanups best.
>
> I would prefer to keep the discussed single function call within
> the basic block of the if statement.
>
> Have we got different opinions about the shown implementation details?
Yes.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <85b948e0-bf37-dff0-6792-77b5dacef9a0@suse.com>]
* [PATCH v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()
[not found] ` <85b948e0-bf37-dff0-6792-77b5dacef9a0@suse.com>
@ 2016-07-20 11:27 ` SF Markus Elfring
2016-07-20 11:30 ` [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree" SF Markus Elfring
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-20 11:27 UTC (permalink / raw)
To: xen-devel, linux-scsi, Jürgen Groß,
David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Jul 2016 13:20:04 +0200
Further update suggestions were taken into account
after a patch was applied from static source code analysis.
Markus Elfring (3):
Delete an unnecessary check before the function call "kfree"
Rename jump labels in scsiback_device_action()
Pass a failure indication as a constant
drivers/xen/xen-scsiback.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
--
2.9.2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"
2016-07-20 11:27 ` [PATCH v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action() SF Markus Elfring
@ 2016-07-20 11:30 ` SF Markus Elfring
2016-07-20 11:44 ` Juergen Gross
2016-07-20 11:34 ` [PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action() SF Markus Elfring
2016-07-20 11:36 ` [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant SF Markus Elfring
2 siblings, 1 reply; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-20 11:30 UTC (permalink / raw)
To: xen-devel, linux-scsi, Jürgen Groß,
David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 19 Jul 2016 15:42:19 +0200
The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2: Rebased on source files from "Linux next-20160719"
drivers/xen/xen-scsiback.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index d6950e0..4a48c06 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -627,8 +627,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
transport_generic_free_cmd(&pending_req->se_cmd, 1);
return;
err:
- if (tmr)
- kfree(tmr);
+ kfree(tmr);
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
}
--
2.9.2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"
2016-07-20 11:30 ` [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree" SF Markus Elfring
@ 2016-07-20 11:44 ` Juergen Gross
0 siblings, 0 replies; 20+ messages in thread
From: Juergen Gross @ 2016-07-20 11:44 UTC (permalink / raw)
To: SF Markus Elfring, xen-devel, linux-scsi, David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
On 20/07/16 13:30, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 19 Jul 2016 15:42:19 +0200
>
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Even if already given, here it is again:
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()
2016-07-20 11:27 ` [PATCH v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action() SF Markus Elfring
2016-07-20 11:30 ` [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree" SF Markus Elfring
@ 2016-07-20 11:34 ` SF Markus Elfring
2016-07-20 11:45 ` Juergen Gross
2016-07-20 11:36 ` [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant SF Markus Elfring
2 siblings, 1 reply; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-20 11:34 UTC (permalink / raw)
To: xen-devel, linux-scsi, Jürgen Groß,
David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Jul 2016 13:03:16 +0200
* Adjust jump targets according to the Linux coding style convention.
* A bit of refactoring for the control flow
Suggested-by: Jürgen Groß <jgross@suse.com>
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2: Rebased on source files from "Linux next-20160719"
Changes from a bit of code review
drivers/xen/xen-scsiback.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 4a48c06..eb274df 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -604,10 +604,8 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
int rc, err = FAILED;
tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
- if (!tmr) {
- target_put_sess_cmd(se_cmd);
- goto err;
- }
+ if (!tmr)
+ goto put_cmd;
init_waitqueue_head(&tmr->tmr_wait);
@@ -616,7 +614,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
unpacked_lun, tmr, act, GFP_KERNEL,
tag, TARGET_SCF_ACK_KREF);
if (rc)
- goto err;
+ goto free_tmr;
wait_event(tmr->tmr_wait, atomic_read(&tmr->tmr_complete));
@@ -626,7 +624,9 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
transport_generic_free_cmd(&pending_req->se_cmd, 1);
return;
-err:
+put_cmd:
+ target_put_sess_cmd(se_cmd);
+free_tmr:
kfree(tmr);
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
}
--
2.9.2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant
2016-07-20 11:27 ` [PATCH v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action() SF Markus Elfring
2016-07-20 11:30 ` [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree" SF Markus Elfring
2016-07-20 11:34 ` [PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action() SF Markus Elfring
@ 2016-07-20 11:36 ` SF Markus Elfring
2016-07-20 11:44 ` Juergen Gross
2 siblings, 1 reply; 20+ messages in thread
From: SF Markus Elfring @ 2016-07-20 11:36 UTC (permalink / raw)
To: xen-devel, linux-scsi, Jürgen Groß,
David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Jul 2016 13:12:33 +0200
Pass the constant "FAILED" in a function call directly instead of
using an intialisation for a local variable.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2: Rebased on source files from "Linux next-20160719"
drivers/xen/xen-scsiback.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index eb274df..fa08ec6 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -601,7 +601,7 @@ static void scsiback_device_action(struct vscsibk_pend *pending_req,
struct se_cmd *se_cmd = &pending_req->se_cmd;
struct scsiback_tmr *tmr;
u64 unpacked_lun = pending_req->v2p->lun;
- int rc, err = FAILED;
+ int rc, err;
tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
if (!tmr)
@@ -628,7 +628,7 @@ put_cmd:
target_put_sess_cmd(se_cmd);
free_tmr:
kfree(tmr);
- scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
+ scsiback_do_resp_with_sense(NULL, FAILED, 0, pending_req);
}
/*
--
2.9.2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant
2016-07-20 11:36 ` [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant SF Markus Elfring
@ 2016-07-20 11:44 ` Juergen Gross
0 siblings, 0 replies; 20+ messages in thread
From: Juergen Gross @ 2016-07-20 11:44 UTC (permalink / raw)
To: SF Markus Elfring, xen-devel, linux-scsi, David Vrabel, Boris Ostrovsky
Cc: Julia Lawall, kernel-janitors, LKML
On 20/07/16 13:36, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 20 Jul 2016 13:12:33 +0200
>
> Pass the constant "FAILED" in a function call directly instead of
> using an intialisation for a local variable.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Even if already given, here it is again:
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread