Leeman,
Thanks very much for your comments. See inline:
On Mon, Apr 16, 2018 at 2:38 PM, The Lee-Man <leeman.dun...@gmail.com>
wrote:
...
>> -retry:
>> - rc = iscsid_exec_req(&req, &rsp, 0, ISCSID_REQ_TIMEOUT);
>> /*
>> - * handle race where iscsid proc is starting up while we are
>> - * trying to connect.
>> + * Need to handle race where iscsid proc is starting up while
we are
>> + * trying to connect. Retry with exponential backoff, start
from 50 ms.
>> */
>> - if (rc == ISCSI_ERR_ISCSID_NOTCONN && retries < 30) {
>> - retries++;
>> - sleep(1);
>> - goto retry;
>> - } else if (rc)
>> - iscsi_err_print_msg(rc);
>> + for (msec = 50; msec <= 15000; msec <<= 1) {
>
>
> Why are you using 50 -> 15000? Isn't the sequence then going to be:
> -> 50
> -> 100
> -> 200
> -> 400
> -> 800
> -> 1600 (too large)
>
> So why not either go to 800 or to 1600?
I saw your another email. Yes, with the old implementation, retry gives up
until 30 seconds, that is why we choose to stop at 30s/2.
>
>
>> + rc = iscsid_exec_req(&req, &rsp, 0, ISCSID_REQ_TIMEOUT);
>> + if (rc == 0) {
>> + return rc;
>> + } else if (rc == ISCSI_ERR_ISCSID_NOTCONN) {
>> + ts.tv_sec = msec / 1000;
>> + ts.tv_nsec = (msec % 1000) * 1000000L;
>> +
>> + /* On EINTR, retry nanosleep with remaining
time. */
>> + while ((err = nanosleep(&ts, &ts)) < 0 &&
>> + errno == EINTR);
>
>
> I'd feel better about this if you used one more level of parenthesis.
>
>
> Also, why are you supplying two timespec values? You don't care about
> the remainder, so pass in a NULL as the second argument?
As the comments explained: on EINTR, we will retry nanosleep with the
remaining time, that is why we care about the remainder. Hopefully this
also explains why we don't need one more level of parenthesis.
>>
>> + if (err < 0)
>> + break;
>> + } else {
>> + break;
>> + }
>> + }
>> +
>> + iscsi_err_print_msg(rc);
>> return rc;
>> }
>>
>
>
> Also, what kind of time improvement did you see with this?
>From what we can see, the interval between below two log messages has been
reduced from 1s to 0.05s.
systemd[689]: Executing: /usr/sbin/iscsistart -i ...
scsi host2: iSCSI Initiator over TCP/IP
Thanks
- Cathy
>>
>> --
>> 2.17.0
>>
> --
> You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to open-iscsi+unsubscr...@googlegroups.com.
> To post to this group, send email to open-iscsi@googlegroups.com.
> Visit this group at https://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
