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, cathy.z...@oracle.com 
> wrote:
>>
>> From: Cathy Zhou <cathy.z...@oracle.com> 
>>
>> iscsi boot performance can be improved by sleep() with finer 
>> grained interval and exponential backoff. 
>>
>> Signed-off-by: Cathy Zhou <cathy.z...@oracle.com> 
>> --- 
>>  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 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.

Reply via email to