Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1444911 > unrolled thread

[PATCH 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()

Started bySF Markus Elfring <elfring@users.sourceforge.net>
First post2016-07-16 22:30 +0200
Last post2016-07-18 07:10 +0200
Articles 16 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 0/3] xen-scsiback: Fine-tuning for scsiback_device_action() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 22:30 +0200
    [PATCH 2/3] xen-scsiback: One function call less in  scsiback_device_action() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 22:30 +0200
      Re: [PATCH 2/3] xen-scsiback: One function call less in  scsiback_device_action() after error detection Juergen Gross <jgross@suse.com> - 2016-07-18 07:20 +0200
        Re: [PATCH 2/3] xen-scsiback: One function call less in  scsiback_device_action() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-19 17:00 +0200
          Re: [PATCH 2/3] xen-scsiback: One function call less in  scsiback_device_action() after error detection Juergen Gross <jgross@suse.com> - 2016-07-20 06:40 +0200
            Re: [PATCH 2/3] xen-scsiback: One function call less in  scsiback_device_action() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-20 07:20 +0200
              Re: [PATCH 2/3] xen-scsiback: One function call less in  scsiback_device_action() after error detection Juergen Gross <jgross@suse.com> - 2016-07-20 07:30 +0200
                [PATCH v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-20 13:30 +0200
                  [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the  function call "kfree" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-20 13:40 +0200
                    Re: [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before  the function call "kfree" Juergen Gross <jgross@suse.com> - 2016-07-20 13:50 +0200
                  [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-20 13:40 +0200
                    Re: [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a  constant Juergen Gross <jgross@suse.com> - 2016-07-20 13:50 +0200
                  [PATCH v2 2/3] xen-scsiback: Rename jump labels in  scsiback_device_action() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-20 13:40 +0200
                    Re: [PATCH v2 2/3] xen-scsiback: Rename jump labels in  scsiback_device_action() Juergen Gross <jgross@suse.com> - 2016-07-20 13:50 +0200
    [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the  function call "kfree" SF Markus Elfring <elfring@users.sourceforge.net> - 2016-07-16 22:30 +0200
      Re: [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the  function call "kfree" Juergen Gross <jgross@suse.com> - 2016-07-18 07:10 +0200

#1444911 — [PATCH 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-16 22:30 +0200
Subject[PATCH 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()
Message-ID<rVEnL-8l8-5@gated-at.bofh.it>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 16 Jul 2016 22:06:54 +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"
  One function call less in scsiback_device_action() after error detection
  Pass a failure indication as a constant

 drivers/xen/xen-scsiback.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

-- 
2.9.1

[toc] | [next] | [standalone]


#1444912 — [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-16 22:30 +0200
Subject[PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
Message-ID<rVEnM-8l8-11@gated-at.bofh.it>
In reply to#1444911
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

[toc] | [prev] | [next] | [standalone]


#1445211 — Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

FromJuergen Gross <jgross@suse.com>
Date2016-07-18 07:20 +0200
SubjectRe: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
Message-ID<rW98d-1ZF-1@gated-at.bofh.it>
In reply to#1444912
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);
>  }
>  
> 

[toc] | [prev] | [next] | [standalone]


#1446531 — Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-19 17:00 +0200
SubjectRe: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
Message-ID<rWEF3-5jj-3@gated-at.bofh.it>
In reply to#1445211
>> @@ -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

[toc] | [prev] | [next] | [standalone]


#1446964 — Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

FromJuergen Gross <jgross@suse.com>
Date2016-07-20 06:40 +0200
SubjectRe: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
Message-ID<rWRsB-55G-3@gated-at.bofh.it>
In reply to#1446531
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

[toc] | [prev] | [next] | [standalone]


#1446974 — Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-20 07:20 +0200
SubjectRe: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
Message-ID<rWS5j-5Cb-1@gated-at.bofh.it>
In reply to#1446964
>>>> @@ -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

[toc] | [prev] | [next] | [standalone]


#1446979 — Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

FromJuergen Gross <jgross@suse.com>
Date2016-07-20 07:30 +0200
SubjectRe: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection
Message-ID<rWSeZ-5Fq-11@gated-at.bofh.it>
In reply to#1446974
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

[toc] | [prev] | [next] | [standalone]


#1447177 — [PATCH v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-20 13:30 +0200
Subject[PATCH v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()
Message-ID<rWXRo-M3-31@gated-at.bofh.it>
In reply to#1446979
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

[toc] | [prev] | [next] | [standalone]


#1447184 — [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-20 13:40 +0200
Subject[PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"
Message-ID<rWY14-P5-7@gated-at.bofh.it>
In reply to#1447177
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

[toc] | [prev] | [next] | [standalone]


#1447191 — Re: [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

FromJuergen Gross <jgross@suse.com>
Date2016-07-20 13:50 +0200
SubjectRe: [PATCH v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"
Message-ID<rWYaJ-Sj-1@gated-at.bofh.it>
In reply to#1447184
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

[toc] | [prev] | [next] | [standalone]


#1447187 — [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-20 13:40 +0200
Subject[PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant
Message-ID<rWY14-P5-15@gated-at.bofh.it>
In reply to#1447177
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

[toc] | [prev] | [next] | [standalone]


#1447192 — Re: [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant

FromJuergen Gross <jgross@suse.com>
Date2016-07-20 13:50 +0200
SubjectRe: [PATCH v2 3/3] xen-scsiback: Pass a failure indication as a constant
Message-ID<rWYaJ-Sj-3@gated-at.bofh.it>
In reply to#1447187
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

[toc] | [prev] | [next] | [standalone]


#1447189 — [PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-20 13:40 +0200
Subject[PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()
Message-ID<rWY14-P5-9@gated-at.bofh.it>
In reply to#1447177
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

[toc] | [prev] | [next] | [standalone]


#1447193 — Re: [PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()

FromJuergen Gross <jgross@suse.com>
Date2016-07-20 13:50 +0200
SubjectRe: [PATCH v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()
Message-ID<rWYaJ-Sj-9@gated-at.bofh.it>
In reply to#1447189
On 20/07/16 13:34, SF Markus Elfring wrote:
> 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>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[toc] | [prev] | [next] | [standalone]


#1444913 — [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2016-07-16 22:30 +0200
Subject[PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"
Message-ID<rVEnM-8l8-13@gated-at.bofh.it>
In reply to#1444911
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

[toc] | [prev] | [next] | [standalone]


#1445207 — Re: [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

FromJuergen Gross <jgross@suse.com>
Date2016-07-18 07:10 +0200
SubjectRe: [PATCH 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"
Message-ID<rW8Yx-1VK-1@gated-at.bofh.it>
In reply to#1444913
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web