[PATCH 1/2] os/connection: Call ClientReady() based on a level trigger rather than an edge trigger

[Jeremy Huddleston Sequoia]

> On Sep 18, 2016, at 09:58, Keith Packard <keithp at keithp.com> wrote:
> 
> Matthieu Herrb <matthieu at herrb.eu> writes:
> 
>> this doesn't fix an issue I'm seeing on OpenBSD with xterm not beeing
>> able to start (it makes the X server spin at 100% CPU in
>> WaitForSomething()), while Jeremy's patches do fix the issue for me.
> 
> I think Jeremy's patch will cause the server to spin while any client is
> not writable, which isn't good.

I don't believe that is the case because ClientReady does:

ospoll_mute(server_poll, fd, X_NOTIFY_WRITE);

If I read the flow right, the level trigger causes WaitForSomething to spin up only when a client is ready to receive data.  ClientReady then mutes the trigger.  FlushClient then possibly re-enables it if we EAGAIN.  If/when the client is ready again (level trigger), ClientReady will fire again.

If the client is not ready to receive data, ClientReady won't fire.

My patch does indeed cause us to needlessly FlushAllOutput() if something else (eg: a new connection, new request) causes WaitForSomething() to spin up, but that's no different than someone currently just setting NewOutputPending=YES even though there is no pending output.

>> I've not been able to really understand the issue yet, but that's a
>> data point. (And I'm glad to see that it doesn't seem to be an OpenBSD
>> specific issue :)
> 
> I'd bet anyone using poll(2) instead of epoll(2) will see the same
> problems. It should be possible to reproduce this on Linux by using the
> poll path instead of epoll.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4465 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160918/344c6898/attachment-0001.bin>