On Monday, April 16, 2018 at 2:38:01 PM UTC-7, The Lee-Man wrote:
>
> On Friday, April 13, 2018 at 3:19:11 PM UTC-7, [email protected]
> wrote:
>>
>> From: Cathy Zhou <[email protected]>
>>
>> iscsi boot performance can be improved by sleep() with finer
>> grained interval and exponential backoff.
>>
>> Signed-off-by: Cathy Zhou <[email protected]>
>> ---
>> usr/iscsistart.c | 35 ++++++++++++++++++++++++-----------
>> 1 file changed, 24 insertions(+), 11 deletions(-)
>>
>> diff --git a/usr/iscsistart.c b/usr/iscsistart.c
>> index 67ac475..1e77e40 100644
>> --- a/usr/iscsistart.c
>> +++ b/usr/iscsistart.c
>> @@ -27,6 +27,7 @@
>> #include <unistd.h>
>> #include <string.h>
>> #include <signal.h>
>> +#include <time.h>
>> #include <sys/mman.h>
>> #include <sys/utsname.h>
>> #include <sys/signal.h>
>> @@ -224,7 +225,8 @@ static int login_session(struct node_rec *rec)
>> {
>> iscsiadm_req_t req;
>> iscsiadm_rsp_t rsp;
>> - int rc, retries = 0;
>> + int rc, msec, err;
>> + struct timespec ts;
>>
>> rc = apply_params(rec);
>> if (rc)
>> @@ -237,18 +239,29 @@ static int login_session(struct node_rec *rec)
>> req.command = MGMT_IPC_SESSION_LOGIN;
>> memcpy(&req.u.session.rec, rec, sizeof(*rec));
>>
>> -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?
>
>
Doh! I am off by a factor of 10!
-> 3200, 6400, 12800
Still, the point is that maybe the max should be the a power of 10 of the
min? (It's really just a nit)
> + 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?
>
>> + if (err < 0)
>> + break;
>> + } else {
>> + break;
>> + }
>> + }
>> +
>> + iscsi_err_print_msg(rc);
>> return rc;
>> }
>>
>>
>
> Also, what kind of time improvement did you see with this?
>
>> --
>> 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 [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
