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?
+ 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.
