[OpenWrt-Devel] [PATCH][RFC] libubus: added max retry mechanism

[Alexandru Ardelean]

Quick info follow-up:  strace-ing on the PIDs (with high CPU usage) shows a
lot of EAGAIN errors from recvfrom() and a lot of recvfrom() + poll() +
clock_gettime_monotonic() calls.

Thinking about it, would it make more sense to add a short wait instead [to
reduce the throttle] and let it "infinitely" loop ?  ["infinitely" loop ==
loop until it does not error with EAGAIN]


On Tue, Jul 29, 2014 at 4:17 PM, Alexandru Ardelean <ardeleanalex at gmail.com>
wrote:

> Seems that every once in a while libubus takes up the whole CPU
> for certain processes.
>
> This could be bad code that I keep (re)writing.
>
> I don't know if it's ok to give up after a certain number of
> retries; high CPU usage could indicate I should look deeper
> at my logic.
>
> Which is why I've submitted this patch for review/opinions/comments.
>
> Can also be reviewed here:
> https://github.com/commodo/ubus/commits/max_retry_mechanism
>
> Thanks
>
> ---
>  libubus-io.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/libubus-io.c b/libubus-io.c
> index 31dad27..6bc06aa 100644
> --- a/libubus-io.c
> +++ b/libubus-io.c
> @@ -29,6 +29,7 @@
>  #define STATIC_IOV(_var) { .iov_base = (char *) &(_var), .iov_len =
> sizeof(_var) }
>
>  #define UBUS_MSGBUF_REDUCTION_INTERVAL 16
> +#define UBUS_MAX_RETRIES               256
>
>  static const struct blob_attr_info ubus_policy[UBUS_ATTR_MAX] = {
>         [UBUS_ATTR_STATUS] = { .type = BLOB_ATTR_INT32 },
> @@ -75,6 +76,7 @@ static int writev_retry(int fd, struct iovec *iov, int
> iov_len, int sock_fd)
>                 .msg_controllen = sizeof(fd_buf),
>         };
>         int len = 0;
> +       int retries = UBUS_MAX_RETRIES;
>
>         do {
>                 int cur_len;
> @@ -91,9 +93,9 @@ static int writev_retry(int fd, struct iovec *iov, int
> iov_len, int sock_fd)
>                         switch(errno) {
>                         case EAGAIN:
>                                 wait_data(fd, true);
> -                               break;
>                         case EINTR:
> -                               break;
> +                               if (retries-- > 0)
> +                                       break;
>                         default:
>                                 return -1;
>                         }
> @@ -115,6 +117,7 @@ static int writev_retry(int fd, struct iovec *iov, int
> iov_len, int sock_fd)
>                 iov->iov_len -= cur_len;
>                 msghdr.msg_iov = iov;
>                 msghdr.msg_iovlen = iov_len;
> +               retries = UBUS_MAX_RETRIES;
>         } while (1);
>
>         /* Should never reach here */
> @@ -170,6 +173,7 @@ static int recv_retry(int fd, struct iovec *iov, bool
> wait, int *recv_fd)
>                 .msg_iov = iov,
>                 .msg_iovlen = 1,
>         };
> +       int retries = UBUS_MAX_RETRIES;
>
>         while (iov->iov_len > 0) {
>                 if (wait)
> @@ -192,11 +196,16 @@ static int recv_retry(int fd, struct iovec *iov,
> bool wait, int *recv_fd)
>                         bytes = 0;
>                         if (uloop_cancelled)
>                                 return 0;
> -                       if (errno == EINTR)
> -                               continue;
> -
> -                       if (errno != EAGAIN)
> +                       switch (errno) {
> +                       case EINTR:
> +                               if (retries-- > 0)
> +                                       continue;
> +                       case EAGAIN:
> +                               if (retries-- > 0)
> +                                       break;
> +                       default:
>                                 return -1;
> +                       }
>                 }
>                 if (!wait && !bytes)
>                         return 0;
> @@ -210,6 +219,7 @@ static int recv_retry(int fd, struct iovec *iov, bool
> wait, int *recv_fd)
>                 iov->iov_len -= bytes;
>                 iov->iov_base += bytes;
>                 total += bytes;
> +               retries = UBUS_MAX_RETRIES;
>         }
>
>         return total;
> --
> 1.8.4.5
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20140729/14be5321/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel