Hello Ben, I know the issue of <tt>socket's buffer fills up, </tt> <tt>but I think uevent_can_discard() totally process in memory,</tt> <tt>it is light-weight and low cpu consumption, and it reduce the </tt> <tt>iteration count in merging, the test result is also good in</tt> <tt>massive multipath devices scene. </tt> <tt>But </tt>if you still stick to it<tt>, I will revert it to uevents </tt> <tt>processing thread, and call it in uevent_dispatch() before </tt> <tt>calling merge_uevq(), </tt>actually, I'm going to do that. <tt>Regards</tt> <tt>Tang Junhui</tt> 发件人: "Benjamin Marzinski" <bmarzins@redhat.com> 收件人: tang.junhui@zte.com.cn, 抄送: tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn, dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com 日期: 2017/01/04 06:38 主题: Re: [dm-devel] [PATCH 08/12] libmultipath: wait one seconds for more uevents in uevent_listen() in uevents burst situations 发件人: dm-devel-bounces@redhat.com <hr noshade> <tt>On Tue, Dec 27, 2016 at 04:03:25PM +0800, tang.junhui@zte.com.cn wrote: > From: tang.junhui <tang.junhui@zte.com.cn> > > The more uevents are merged, the higher efficiency program will performs. > So, do not process uevents after receiving immediately in uevents burst > situations, but continue wait 1 seconds for more uevents except that too > much uevents (2048) have already been received or too much time eclipse > (60 seconds). The issue I have with doing this in uevent_listen is that we seperated receiving the uevents from servicing the uevents specificially to make sure what we received them as fast as possible. The udev monitor code is all based on a NETLINK socket. If our socket's receive buffer fills up, there is no flow control. Events just start getting dropped, which does cut down on processing time, but not in a way we would like. This issue applies to a lesser extent to you previous two patches. I don't really see the benefit of making sure that we drop the uevents as soon as possible. As long as we don't process them, that should be good enough, right? Now, maybe you put a lot of thought into your timeouts, and feel confident that we will start processing well before the receive buffer fills up. But if so, some comments on that would be reassuring, because from the commit message, they seem fairly arbitrary to me. -Ben > > Change-Id: I763d491540e8114a81d12d603281540a81502742 > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn> > --- > libmultipath/uevent.c | 35 +++++++++++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c > index cc10d65..b0b05e9 100644 > --- a/libmultipath/uevent.c > +++ b/libmultipath/uevent.c > @@ -39,6 +39,7 @@ > #include <linux/netlink.h> > #include <pthread.h> > #include <sys/mman.h> > +#include <sys/time.h> > #include <libudev.h> > #include <errno.h> > > @@ -490,11 +491,37 @@ struct uevent *uevent_from_udev_device(struct udev_device *dev) > return uev; > } > > +bool uevent_burst(struct timeval *start_time, int events) > +{ > + struct timeval diff_time, end_time; > + unsigned long speed; > + unsigned long eclipse_ms; > + > + gettimeofday(&end_time, NULL); > + timersub(&end_time, start_time, &diff_time); > + > + eclipse_ms = diff_time.tv_sec * 1000 + diff_time.tv_usec / 1000; > + if (eclipse_ms == 0) > + return true; > + /* max wait 60s */ > + if (eclipse_ms > 60*1000) { > + condlog(1, "burst continued =%lu ms, stoped", eclipse_ms); > + return false; > + } > + > + speed = (events * 1000) / eclipse_ms; > + if (speed > 10) > + return true; > + > + return false; > +} > + > int uevent_listen(struct udev *udev) > { > int err = 2; > struct udev_monitor *monitor = NULL; > int fd, socket_flags, events; > + struct timeval start_time; > int need_failback = 1; > int timeout = 30; > LIST_HEAD(uevlisten_tmp); > @@ -548,6 +575,7 @@ int uevent_listen(struct udev *udev) > } > > events = 0; > + gettimeofday(&start_time, NULL); > while (1) { > struct uevent *uev; > struct udev_device *dev; > @@ -562,7 +590,8 @@ int uevent_listen(struct udev *udev) > errno = 0; > fdcount = poll(&ev_poll, 1, poll_timeout); > if (fdcount && ev_poll.revents & POLLIN) { > - timeout = 0; > + timeout = uevent_burst(&start_time, events + 1) ? 1:0; > + > dev = udev_monitor_receive_device(monitor); > if (!dev) { > condlog(0, "failed getting udev device"); > @@ -578,7 +607,8 @@ int uevent_listen(struct udev *udev) > } > list_add_tail(&uev->node, &uevlisten_tmp); > events++; > - continue; > + if(events < 2048) > + continue; > } > if (fdcount < 0) { > if (errno == EINTR) > @@ -600,6 +630,7 @@ int uevent_listen(struct udev *udev) > pthread_mutex_unlock(uevq_lockp); > events = 0; > } > + gettimeofday(&start_time, NULL); > timeout = 30; > } > need_failback = 0; > -- > 2.8.1.windows.1 > -- dm-devel mailing list dm-devel@redhat.com </tt><a href="https://www.redhat.com/mailman/listinfo/dm-devel"><tt>https://www.redhat.com/mailman/listinfo/dm-devel</tt></a><tt> </tt>