Date: Fri, 8 Jan 2016 20:59:59 -0500 (EST) From: Rick Macklem <rmacklem@uoguelph.ca> To: Jeff Darcy <jdarcy@redhat.com> Cc: Raghavendra G <raghavendra@gluster.com>, freebsd-fs <freebsd-fs@freebsd.org>, Hubbard Jordan <jkh@ixsystems.com>, Xavier Hernandez <xhernandez@datalab.es>, Gluster Devel <gluster-devel@gluster.org> Subject: Re: [Gluster-devel] FreeBSD port of GlusterFS racks up a lot of CPU usage Message-ID: <981529129.154244852.1452304799182.JavaMail.zimbra@uoguelph.ca> In-Reply-To: <1924941590.6473225.1452248249994.JavaMail.zimbra@redhat.com> References: <571237035.145690509.1451437960464.JavaMail.zimbra@uoguelph.ca> <20151230103152.GS13942@ndevos-x240.usersys.redhat.com> <2D8C2729-D556-479B-B4E2-66E1BB222F41@ixsystems.com> <1083933309.146084334.1451517977647.JavaMail.zimbra@uoguelph.ca> <CADRNtgStOg8UZfxNt-SzvvPf7d1J7CC_gi49ww3BbixU0Ey-rg@mail.gmail.com> <568F6D07.6070500@datalab.es> <CADRNtgRM17Eg3Z=LWifVNo=ai72dMiEVRKS3RwNfQ-dK7Pspew@mail.gmail.com> <1924941590.6473225.1452248249994.JavaMail.zimbra@redhat.com>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
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@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org"
>
[-- Attachment #2 --]
--- 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;
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?981529129.154244852.1452304799182.JavaMail.zimbra>