Messages in this thread | | | Subject | Re: [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection | From | Juergen Gross <> | Date | Wed, 20 Jul 2016 06:35:40 +0200 |
| |
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
| |