Re: FreeBSD port of GlusterFS racks up a lot of CPU usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff Darcy wrote:
> > > I don't know anything about gluster's poll implementation so I may
> > > be totally wrong, but would it be possible to use an eventfd (or a
> > > pipe if eventfd is not supported) to signal the need to add more
> > > file descriptors to the poll call ?
> > >
> > >
> > > The poll call should listen on this new fd. When we need to change
> > > the fd list, we should simply write to the eventfd or pipe from
> > > another thread.  This will cause the poll call to return and we will
> > > be able to change the fd list without having a short timeout nor
> > > having to decide on any trade-off.
> > 
> >
> > Thats a nice idea. Based on my understanding of why timeouts are being
> > used, this approach can work.
> 
> The own-thread code which preceded the current poll implementation did
> something similar, using a pipe fd to be woken up for new *outgoing*
> messages.  That code still exists, and might provide some insight into
> how to do this for the current poll code.
I took a look at event-poll.c and found something interesting...
- A pipe called "breaker" is already set up by event_pool_new_poll() and
  closed by event_pool_destroy_poll(), however it never gets used for anything.

So, I added a few lines of code that writes a byte to it whenever the list of
file descriptors is changed and read when poll() returns, if its revents is set.
I also changed the timeout to -1 (infinity) and it seems to work for a trivial
test.
--> Btw, I also noticed the "changed" variable gets set to 1 on a change, but
    never reset to 0. I didn't change this, since it looks "racey". (ie. I
    think you could easily get a race between a thread that clears it and one
    that adds a new fd.)

A slightly safer version of the patch would set a long (100msec ??) timeout instead
of -1.

Anyhow, I've attached the patch in case anyone would like to try it and will
create a bug report for this after I've had more time to test it.
(I only use a couple of laptops, so my testing will be minimal.)

Thanks for all the help, rick

> _______________________________________________
> freebsd-fs@xxxxxxxxxxx mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe@xxxxxxxxxxx"
> 
--- glusterfs-3.7.6/libglusterfs/src/event-poll.c.sav	2016-01-06 15:58:03.522286000 -0500
+++ glusterfs-3.7.6/libglusterfs/src/event-poll.c	2016-01-08 17:45:28.078024000 -0500
@@ -180,6 +180,16 @@ event_pool_new_poll (int count, int even
         return event_pool;
 }
 
+static void
+event_pool_changed (struct event_pool *event_pool)
+{
+
+        event_pool->changed = 1;
+        /* Write a byte into the breaker pipe to wake up poll(). */
+        if (event_pool->breaker[1] >= 0)
+                write(event_pool->breaker[1], "X", 1);
+}
+
 
 static int
 event_register_poll (struct event_pool *event_pool, int fd,
@@ -244,7 +254,7 @@ event_register_poll (struct event_pool *
                         break;
                 }
 
-                event_pool->changed = 1;
+                event_pool_changed(event_pool);
 
         }
 unlock:
@@ -275,7 +285,7 @@ event_unregister_poll (struct event_pool
                 }
 
                 event_pool->reg[idx] =  event_pool->reg[--event_pool->used];
-                event_pool->changed = 1;
+                event_pool_changed(event_pool);
         }
 unlock:
         pthread_mutex_unlock (&event_pool->mutex);
@@ -350,7 +360,7 @@ event_select_on_poll (struct event_pool 
                 }
 
                 if (poll_in + poll_out > -2)
-                        event_pool->changed = 1;
+                        event_pool_changed(event_pool);
         }
 unlock:
         pthread_mutex_unlock (&event_pool->mutex);
@@ -448,6 +458,7 @@ event_dispatch_poll (struct event_pool *
         int              size = 0;
         int              i = 0;
         int              ret = -1;
+        char             x;
 
         GF_VALIDATE_OR_GOTO ("event", event_pool, out);
 
@@ -472,7 +483,7 @@ event_dispatch_poll (struct event_pool *
                 size = event_dispatch_poll_resize (event_pool, ufds, size);
                 ufds = event_pool->evcache;
 
-                ret = poll (ufds, size, 1);
+                ret = poll (ufds, size, -1);
 
                 if (ret == 0)
                         /* timeout */
@@ -482,7 +493,13 @@ event_dispatch_poll (struct event_pool *
                         /* sys call */
                         continue;
 
-                for (i = 0; i < size; i++) {
+                if (ufds[0].revents != 0 && event_pool->breaker[0] >= 0) {
+                        /* Just read all the junk in the breaker pipe. */
+                        while (read(event_pool->breaker[0], &x, 1) > 0)
+                                ;
+                }
+
+                for (i = 1; i < size; i++) {
                         if (!ufds[i].revents)
                                 continue;
 
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://www.gluster.org/mailman/listinfo/gluster-devel

[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux