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

Groups > linux.kernel > #1446974

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

Show all headers | View raw

>>>> @@ -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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[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

csiph-web