[PATCH] s3:events: Call all ready fd event handlers on eachiteration of the main loop

[Jeremy Allison]

On Thu, Sep 16, 2010 at 02:10:47PM -0700, Steven Danneman wrote:
> > On Mon, Jul 05, 2010 at 06:01:17PM -0700, Steven Danneman wrote:
> > > Attached is a proposed patch to make the main message loop in S3
> more
> > > fair to fd based events.  Currently, network traffic can starve out
> > > all other fd based events. We hit this at Isilon with our fd based
> > > oplock upcalls, but this issue should also affect iNotify on Linux
> > and
> > > oplocks on Irix.
> > >
> > > As this is a major change to the server I wanted to pass it by the
> > > list first.
> > 
> > Sorry, but I think this is the wrong fix. We deliberately only handle
> > one single event in run_events. The reason is that while handling
> > events the state of other pending events can change. This has caused
> us
> > big pain in the past.
> > 
> > Yes, this is indeed a problem that I had to badly work around with
> > d5cf648, but the fix needs to look a bit differently I think. We need
> > to remove a file descriptor off the list of interested fd's whenever
> it
> > is being handled.
> > When either no fd's are left or in a select call nobody is active, we
> > need to push all ones on the list again. This should give us round-
> > robin handling of fds.
> > 
> > I am not sure if only select is affected. It might be the case that
> > Linux under epoll is already fair in the sense that it never starves
> > sockets.
> > 
> > Anybody around knowing for sure?
> > 
> > Volker
> 
> Hey Volker,
> 
> Taking the design considerations you mentioned I've reworked my patch.
> It is not the Cadillac solution, as it only handles the smbd/select
> case, but I went for minimal code disruption.
> 
> This patch effectively changes the select loop in smbd child process
> only, to safely iterate through all fds returned by select each time,
> while still handling timed and signal events round-robin.
> 
> This passes make test and I'm throwing more intensive work loads at it
> now to test it's stability.
> 
> I've also opened bug 7686 to track this issue.  Please tell me what you
> think.

At first glance this looks like a nice fix. Just my 2 cents. Will
test it more soon :-).

Jeremy.