Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-25 Thread Keith Packard
Around 20 o'clock on Sep 25, Juliusz Chroboczek wrote:

 JG So you still have to do a select/poll, read all pending input
 JG events on all different devices, and merge the events in the 
 JG server into a single stream in time stamp order.
 
 Can that happen in dix, or should it be below?

It cannot happen in DIX as there is no queueing at that level, but it 
could easily happen in mieq.

-keith


___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-25 Thread Keith Packard
Around 20 o'clock on Sep 25, Juliusz Chroboczek wrote:

 JG So you still have to do a select/poll, read all pending input
 JG events on all different devices, and merge the events in the 
 JG server into a single stream in time stamp order.
 
 Can that happen in dix, or should it be below?

(responding to self greatly discouraged, but still...)

Actually, because the event timestamps visible in the server are so coarse
(1ms), it might be better to perform the reordering using the kernel event
timestamps.  This could be done by having the input processing code read a 
single event from each input device and selecting the order to enqueue 
them by kernel timestamps.

-keith


___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-25 Thread Alex Deucher
This is somewhat off topic, but as I recall someone posted a patch to
use the linux event input interface to support hotplugging in xfree86.
it may be of use with what you are looking at now...or not. 

Alex

--- Juliusz Chroboczek [EMAIL PROTECTED] wrote:
 JG So I'd just open the device and see if PS/2 mice work at this
 date.
 
 PS/2 mice and AT keyboards most definetily do work with the event
 interface, at least since 2.5.73 (the first 2.5 kernel I tested[1]).
 They do not work in current 2.4, where the event interface is for USB
 peripherals only.
 
 JG So it is entirely possible everything is already there in current
 JG Linux distributions.
 
 It's not in stock 2.4 yet.
 
 JG So you still have to do a select/poll, read all pending input
 JG events on all different devices, and merge the events in the 
 JG server into a single stream in time stamp order.
 
 Can that happen in dix, or should it be below?
 
 (Additionally, a large jump in time, say more than a second, should
 be
 treated as a barrier so as to deal with the clock being stepped. 
 That
 would make one of the hacks in our hw layer go away.)
 
 Juliusz
 
 [1] It did work, but froze when compiling XFree86.  2.6.0-test4
 works but the disk scheduler is disappointing, at least on my
 hardware.


__
Do you Yahoo!?
The New Yahoo! Shopping - with improved product search
http://shopping.yahoo.com
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-24 Thread Ivan Pascal

Keith Packard wrote:
 Yes, it looks like you're correct -- every example I could find would 
 ensure that pending device input was marked in checkForInput.  There are 
 some odd examples of this however, which are somewhat instructive to 
 examine:
 
  a)   The original A/UX server had checkForInput marked from the SIGIO 
   handler and read device events in ProcessInputEvents.

If WaitForSomething returns after the checkForInput check allowing
ProcessInputEvents be executed before the timers check, this case changes
nothing.  It doesn't matter when the events reading was done, before the
ProcessInputEvents or during its execution.

BTW I forgot to mention one other advantage of checkForInput.  It reflects
the events queued from SIGIO handlers but the devicesReadable check misses
such events.

  b)   Many old servers never set checkForInput.  The default values
   point at two different values, so ProcessInput events would be 
   called each time around the Dispatch loop.

Unfortunately, if we assume that some events can be queued from SIGIO handlers
this case is hopeless completely.
With conditions:
- timers should not be processed if there are input events queued but not
  processed by ProcessInputEvents,
- checkForInput can't be used because it always true and with such check
  the timers will never be processed,
- devicesReadable check doesn't reflect input events queued in SIGIO handlers
  and can't be used because it doesn't fulfil the first condition
the problem becomes unsolvable.

  Is it possible to check changes in the file descriptors set instead of just
  the fact that some timer was processed?
 
 That would only be necessary if performance across timer execution were a 
 critical factor -- essentially what we're forcing by returning 0 is 
 another call to select, which will do all of the file descriptor checking 
 necessary.  It seems simpler to let the existing code handle this case.

OK.  I made the next (and hope it's final) version.
In both branches (after the select) the subroutine returns if at least one
time is expired.

-- 
 Ivan U. Pascal |   e-mail: [EMAIL PROTECTED]
   Administrator of |   Tomsk State University
 University Network |   Tomsk, Russia
--- xc/programs/Xserver/os/WaitFor.c.orig   Wed Sep 17 17:24:12 2003
+++ xc/programs/Xserver/os/WaitFor.cWed Sep 24 19:15:53 2003
@@ -131,17 +131,12 @@
  * pClientsReady is an array to store ready client-index values into.
  */
 
-static INT32 timeTilFrob = 0;  /* while screen saving */
-
 int
 WaitForSomething(int *pClientsReady)
 {
 int i;
 struct timeval waittime, *wt;
 INT32 timeout = 0;
-#ifdef DPMSExtension
-INT32 standbyTimeout = 0, suspendTimeout = 0, offTimeout = 0;
-#endif
 fd_set clientsReadable;
 fd_set clientsWritable;
 int curclient;
@@ -188,138 +183,17 @@
else
{
 #endif
-#ifdef DPMSExtension
-   if (ScreenSaverTime  0 || DPMSEnabled || timers)
-#else
-   if (ScreenSaverTime  0 || timers)
-#endif
-   now = GetTimeInMillis();
-   wt = NULL;
+wt = NULL;
if (timers)
-   {
-   while (timers  (int) (timers-expires - now) = 0)
-   DoTimer(timers, now, timers);
-   if (timers)
-   {
-   timeout = timers-expires - now;
-   waittime.tv_sec = timeout / MILLI_PER_SECOND;
-   waittime.tv_usec = (timeout % MILLI_PER_SECOND) *
-   (100 / MILLI_PER_SECOND);
-   wt = waittime;
-   }
-   }
-   if (ScreenSaverTime  0
-#ifdef DPMSExtension
-   || (DPMSEnabled 
-(DPMSStandbyTime  0 || DPMSSuspendTime  0 || DPMSOffTime  0))
-#endif
-   ) {
-#ifdef DPMSExtension
-   if (ScreenSaverTime  0)
-#endif
-   timeout = (ScreenSaverTime -
-  (now - lastDeviceEventTime.milliseconds));
-#ifdef DPMSExtension
-   if (DPMSStandbyTime  0)
-   standbyTimeout = (DPMSStandbyTime -
- (now - lastDeviceEventTime.milliseconds));
-   if (DPMSSuspendTime  0)
-   suspendTimeout = (DPMSSuspendTime -
- (now - lastDeviceEventTime.milliseconds));
-   if (DPMSOffTime  0)
-   offTimeout = (DPMSOffTime -
- (now - lastDeviceEventTime.milliseconds));
-#endif /* DPMSExtension */
-
-   if (
-   timeout = 0
-#ifdef DPMSExtension
- ScreenSaverTime  0
-#endif /* DPMSExtension */
-   ) {
-   INT32 timeSinceSave;
-
-   timeSinceSave = -timeout;
-   if (timeSinceSave = timeTilFrob  timeTilFrob = 0)
-   {
-   ResetOsBuffers(); /* not ideal, but better than nothing */
-   SaveScreens(SCREEN_SAVER_ON, ScreenSaverActive);
-#ifdef DPMSExtension
-   if 

Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-24 Thread Juliusz Chroboczek
KP  a) The original A/UX server had checkForInput marked from the SIGIO 
KP handler and read device events in ProcessInputEvents.

Why?

Juliusz
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-24 Thread Keith Packard
Around 15 o'clock on Sep 24, Juliusz Chroboczek wrote:

 KP  a)   The original A/UX server had checkForInput marked from the SIGIO 
 KP   handler and read device events in ProcessInputEvents.
 
 Why?

There are two goals when dealing with input in the X server:

1)  Notice available input quickly, process it as soon as possible
2)  Timestamp events accurately

Without asynchronous notification (SIGIO), the X server will poll file
descriptors for input devices when it polls for input from clients.  With
the time-based scheduler, this happens every 20ms+ while the server is 
busy.  Any kind of asynchronous notification that can make the 
checkForInput values change will cause the server to suspend request 
processing after the current request and call ProcessInputEvents.  That 
reduces the latency to no more than one request time, generally much less 
than 20ms+.

So, the simple SIGIO-variable method was sufficient to eliminate as much
event processing latency as is practical in the X server -- checkForInput
would change, and the server would call ProcessInputEvents immediately.  No
extra event queue was needed as events could be dumped straight into DIX.

However, if the kernel events don't include a timestamp, you end up
timestamping them when they're read, which will introduce significant
errors.  I fixed this in R5 by writing the mi event queue code which could 
read input at SIGIO time, timestamp events correctly and then process them 
at ProcessInputEvents time.  This was mostly done so that the sprite would 
track the mouse more accurately.

An additional benefit of reading at SIGIO time is that event streams are 
more likely to be interleaved correctly.  Delaying the read until 
ProcessInputEvents may let events from multiple devices queue up in the 
kernel; the X server then has no idea what order they originally occured 
in; older code would simply read all events from one device before looking 
at another.

Better yet would be to have the kernel timestamp events with a 
high-precision timer so that the X server could insert them into the event 
queue in proper order; that would improve event handling even if events 
were read at SIGIO time.

-keith


___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-24 Thread Keith Packard
Around 19 o'clock on Sep 24, Juliusz Chroboczek wrote:

 Linux 2.[56] does that on the new /dev/event interface (/dev/psaux
 would appear to be deprecated).  Is it worth using?

Yes.  Aside from the improved quality of information available, using 
a standard interface for all devices will dramatically simplify the input 
code.

Note that 2.4 provides /dev/input/event* for USB input devices; 2.6 simply 
extends that to cover 'legacy' devices.

-keith


___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-24 Thread Juliusz Chroboczek
JG In fact, the event interface exists in current 2.4; not
JG just 2.[45]. Look in /dev/input/event*.

Some of us still use PS/2 mice...

JG Note that this interface is not just for mice, but is generalized
JG to be usable by pretty arbitrary input devices,

Yes; it looks like all input devices go through the single event
interface in 2.6.  That would seem to guarantee correct ordering of
events from different devices.

Juliusz
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-24 Thread Keith Packard
Around 1 o'clock on Sep 25, Juliusz Chroboczek wrote:

 Some of us still use PS/2 mice...

Nearly all laptops still have the touch pad connected through the PS/2 
port, so until the 2.6 driver is backported to 2.4, we'll need PS/2 
support in X servers.

 Yes; it looks like all input devices go through the single event
 interface in 2.6.  That would seem to guarantee correct ordering of
 events from different devices.

Not automatically; it still requires sorting events from multiple event
streams as each device is presented as a separate file, however the 
timestamps present in each event make it possible to do this correctly.

Old Ultrix machines actually merged the keyboard and mouse event streams 
within the kernel and presented a memory-mapped event queue.  This was 
convenient from the X server's perspective, but seems rather hard to 
manage with the wealth of weird input devices available these days, not to 
mention supporting multiple users on a single machine.

-ketih


___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-24 Thread Jim Gettys
I use both USB and PS/2.  I did  play with the event interface
a few hours last year (before my latest encounter with a surgeon),
and convinced myself it seemed to be working correctly with USB.
I think there are patches (that may or may not have been merged
into current 2.4, I don't remember) to add the legacy support.
So I'd just open the device and see if PS/2 mice work at this date.

A lot of goodies developed initially in the development
stream of Linux get backported to the stable series.  So it
is entirely possible everything is already there in current
Linux distributions.  But I've not had time to check myself...

I don't think it is a single event queue: there is
an event queue/device, IIRC.  I think I initially argued for a single
queue, but the flexibility of having different protection on
a per device basis made that argument specious in the design
discussion, and I acknowledged this was a very bad idea...

So you still have to do a select/poll, read all pending input
events on all different devices, and merge the events in the 
server into a single stream in time stamp order.  
At most a page or two of code, -I believe (to do the merge).

But all the different event types go through a single 
(narrow) interface.  Multi-coordinate devices generate multiple
events at once (one event/coordinate).
   - Jim


On Wed, 2003-09-24 at 19:35, Juliusz Chroboczek wrote:
 JG In fact, the event interface exists in current 2.4; not
 JG just 2.[45]. Look in /dev/input/event*.
 
 Some of us still use PS/2 mice...
 
 JG Note that this interface is not just for mice, but is generalized
 JG to be usable by pretty arbitrary input devices,
 
 Yes; it looks like all input devices go through the single event
 interface in 2.6.  That would seem to guarantee correct ordering of
 events from different devices.
 
 Juliusz
 ___
 Devel mailing list
 [EMAIL PROTECTED]
 http://XFree86.Org/mailman/listinfo/devel
-- 
Jim Gettys [EMAIL PROTECTED]
HP Labs, Cambridge Research Laboratory

___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-23 Thread Ivan Pascal
Egbert Eich wrote:
   The XKB code could call ProcessInputEvents just above the check for a 
   pending repeat key; that would ensure that any input queued either in the 
   X server or the kernel would get processed before repeat status was 
   checked.
   
 Hm, doesn't ProcessInputEvents() only process the events already in
 the queue on the server? From looking at the code it doesn't seem
 to do a read() to obtain events that have not been read from the
 kernel - but that's what we'd need.

Right.  ProcessInputEvents doesn't read events from the kernel.
Such check in the autorepeat callback function might make sense only being used
with a SIGIO driven input.

In any case, the current spot isn't sufficient as the timers 
   will never be executed if the server is constantly receiving application 
   input, so at least that needs to be fixed.
   
 
 One could move the timer handlers past the end of the 
 if (i = 0) { } else { } statements. However with input queued
 (ie when the mouse is constantly moved) the timers don't get
 called either.

Can the mouse movement really make such frequent stream of events?
BTW for timers that need the devices input checked first (screensaver, DPMS,
keys autorepeat) it is not a problem.  Anyway they do something useful when
input evens absent some time.  But for other timers it can be important.
If such danger really exists it argues for a making two kinds of timers and
distinguishing them.

-- 
 Ivan U. Pascal |   e-mail: [EMAIL PROTECTED]
   Administrator of |   Tomsk State University
 University Network |   Tomsk, Russia
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-23 Thread Ivan Pascal
 Around 21 o'clock on Sep 22, Ivan Pascal wrote:

  Also I added a flag for timers TimerNeedsCheckInput.  The timers without this
  flag are processed before the select all others are delaied until the second
  timers check.  (The second check doesn't distinguish those timers.)
 
 I think that's overkill; the simple expediency of returning 0 from 
 WaitForSomething after a timer has been processed will make sure that any 
 effects of the timer are reflected at every scheduling stage.
 
  WakeupHandler(i, (pointer)LastSelectMask);
  +ProcessInputEvents();
 
 Calling ProcessInputEvents from WaitForSomething is not appropriate; 
 input events are already processed by dispatch at the appropriate time.  

I agree.  It was a silly kludge.

 Instead of attempting to patch the code to kludge things into working, 
 let's try to nail down precisely how things should work and then rewrite 
 the code to match that.  Here's a short list of 'requirements':

Thank you.  Offering my patch I want to hear an opinion of those who know
well how this loop works.

  1)   DoTimer called only when all pending input processed.
 
   a)  After select
   b)  After checkForInput tested
Agree.

   c)  After devicesReadable is checked
But here I don't understand why this check is needed.
If the select returns some readable devices fd's they all should be read
in Wakeup handlers immediately after the select and this reading should be
reflected in checkForInput.  Moreover it can happen that a driver reads events
from the kernel but doesn't put any events into the servers queue (for example
the keyboard driver that processes VT- or VMode-swithing internally).

  2)   DoTimer called even if server swamped with client input
 
   a)  Must check timers even when select returns positive
Agree.

  3)   DoTimer effects on file descriptors respected (not that any known
   instances exist)
 
   a)  Return 0 after any timers processed

Is it possible to check changes in the file descriptors set instead of just
the fact that some timer was processed?
I imagine something like:
  save AllSockets
  do timers
  if XFD_ANYSET in (saved_AllSockets XOR AllSockets)
 return 0;

Or what do you mean?

Also I think it is not necessary to return in the branch 'the select returned 0'
because in this case there are not any action with sockets and WaitForSomething
returns in loop back to the select.

  4)   Minimal system calls in normal case
 
   a)  Call GetTimeInMillis() as few times as possible
   b)  Don't call ProcessInputEvents from WaitForSomething
Agree.

The next version of the patch attached (WaitFor.c only).
I didn't make the 'return after the timers processing' but just put a comment
where I think it should be done.

-- 
 Ivan U. Pascal |   e-mail: [EMAIL PROTECTED]
   Administrator of |   Tomsk State University
 University Network |   Tomsk, Russia
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-23 Thread Keith Packard
Around 21 o'clock on Sep 23, Ivan Pascal wrote:

  c)  After devicesReadable is checked
 But here I don't understand why this check is needed.
 If the select returns some readable devices fd's they all should be read
 in Wakeup handlers immediately after the select and this reading should be
 reflected in checkForInput.

Yes, it looks like you're correct -- every example I could find would 
ensure that pending device input was marked in checkForInput.  There are 
some odd examples of this however, which are somewhat instructive to 
examine:

 a) The original A/UX server had checkForInput marked from the SIGIO 
handler and read device events in ProcessInputEvents.

 b) Many old servers never set checkForInput.  The default values
point at two different values, so ProcessInput events would be 
called each time around the Dispatch loop.

 Moreover it can happen that a driver reads events from the kernel but
 doesn't put any events into the servers queue (for example the keyboard
 driver that processes VT- or VMode-swithing internally).

Whether any events end up getting enqueued isn't relevant; the issue is 
making sure the keyboard driver is given higher priority than the timers 
so that any pending processing is performed before the pending timer is 
executed.  In essence, we're defining a static priority scheme to schedule 
between the three operations:

1)  Device input
2)  Timers
3)  Client requests

Whatever check is deemed sufficient to detect pending device input must be 
performed before the timers are checked; if the only check necessary is 
looking at the checkForInput values, then that's all we need to do.

  a)  Return 0 after any timers processed
 
 Is it possible to check changes in the file descriptors set instead of just
 the fact that some timer was processed?

That would only be necessary if performance across timer execution were a 
critical factor -- essentially what we're forcing by returning 0 is 
another call to select, which will do all of the file descriptor checking 
necessary.  It seems simpler to let the existing code handle this case.

However, it does somewhat beg the question of what it means for 
WaitForSomething to return 0.  Right now, the only case is when there is 
pending input to process and no clients are ready.

 Also I think it is not necessary to return in the branch 'the select returned 0'
 because in this case there are not any action with sockets and WaitForSomething
 returns in loop back to the select.

To some degree, it hardly matters -- you'll see that having
WaitForSomething return 0 causes Dispatch to simply loop back to the top
and call WaitForSomething again.  I think it's a balance between concise 
semantics for WaitForSomething and clearly understandable code inside each 
function.

-keith


___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-22 Thread Ivan Pascal
  Hello,

Egbert Eich wrote:
   One use for timeouts is to handle external network events, such as font 
   server reconnect or XDMCP messages.  In those cases, the timeout routine 
   can modify which sockets will be needed in the select mask, hence the 
   desire for the timeout routine to execute before the select (and various 
   BlockHandler) routines are invoked.  When the timeout routine fires only 
   after select returns, the server will go through a complete scheduling 
   interval before the BlockHandler is invoked.
   
 
 So what would be the way out? I suppose one could use a wakeup handler
 instead. This however would only work if the wakeup handler for keyboard 
 input was called before the xkb timer handler. The latencies of
 other handlers that are called before the xkb handler could also
 cause the timer to expire with the key release unnoticed.
 
 The only way to ensure that a possible release event is detected is
 to read the keyboard in the xkb callback handler to check for 
 pending release events. However I'm afraid xkb simply doesn't work 
 that way.
 
 Another solution may be a a SIGIO driven keyboard input.

Except the solution I offered I considered as possible solutions:
- a check for input directly in the autorepeat timer callback function,
- a using of additional wakeup handler,
- a using SIGIO driven input,
- an additional flag for timers to distinguish them in the first check
  in WaitForSomething.

1. Check for input in the callback function.
I don't like it because it works on the level that is far from an interface
with a kernel.  In our case a release event is not even read from a kernel
driver hence the callback routine should somehow start this reading.  But it
deals with a dix level device structure _DeviceIntRec.  That structure has
a pointer to an input device driver but the driver itself is opaque for this
level.  Moreover in the case of a built-in keyboard driver the DeviceIntRecord
even has not such pointer and so has not any hooks to initiate a reading.
Also we need to be sure that a file to the kernel driver is in NONBLOCK mode.
In most cases it is but I'm not sure it is always.
And finaly, the state when unneeded events are generated happens very seldom
but to avoid it the callback routine has to call a read syscall every time.

2. An additional wakeup handler.
Frankly speaking I didn't consider it deeply because it seemed to me unnecessary
complex.  What happens if there are not real key events and the autorepeat
event is really wanted?  The timer check before the select doesn't emit
an event but only puts a task for a wakeup handler that will be executed after
the select.  But since the timer is already expired, WaitForSomething thinks
this timeout already consumed and makes the select wait until a next timeout
expire or any other event happens.  It is incorrect too.
Therefore in this way we need: change a timeout figuring out, add a wakeup
handler itself (and we need to be sure it stands in queue after input devices
reading), insert a ProcessInputEvents() somewhere between handlers processing
and so on.  And anyway WaitForSomething has to be changed.

3. SIGIO driven input.
It is almost ideal.  Frankly speaking I didn't guess how to solve the problem
that a SIGIO handler only reads input devices but doesn't do a complete
event processing.  But now after reading replays to my message I see that it
is quite simple.  The callback routine can call ProcessInputEvents.  It is
much simpler than to make a driver read events from a kernel and doesn't
consume much time if a queue is empty.
But there is another issue.  As far as I understand not all target platforms
now support SIGIO input.  Therefore it will be a solution for Linux and BSD
only. Is not it?

4. Distinguishing the timers that can be processed before the select and
the timers that should not.

Although nobody mentioned it I still think it could be a solution.  I mean
we can add additional flag to a timer (or more if it is needed).
BTW a subroutine TimerSet already has a 'flags' argument but doesn't save it
for a future use.  On the other hand it means we don't need to change all
TimeSet calls but only those that really need to be checked after an input
devices reading.
The first timers check in WaitForSomething should check this flag in a timer
and skip such timers.

-- 
 Ivan U. Pascal |   e-mail: [EMAIL PROTECTED]
   Administrator of |   Tomsk State University
 University Network |   Tomsk, Russia
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-22 Thread Ivan Pascal

I made new version of the patch.
The issue Keith pointed is fixed.

Also I added a flag for timers TimerNeedsCheckInput.  The timers without this
flag are processed before the select all others are delaied until the second
timers check.  (The second check doesn't distinguish those timers.)

Also I found another issue.  The thing is that the procedure xf86Wakeup that
reads input devices processes only one device in a turn because, as it is said
in comments, it Must break [loop] here because more than one device may share
the same file descriptor.  Now the built-in keyboard driver is called
separately before the loop and all works well.  But if a keyboard is served
by a module driver and there are more than one devices with 'ready for reading'
events there is not guaranty that all input devices events will be read before
the timers checking.

I made a separate patch for xf86Wakeup (in a separate attachment).
Please review.

-- 
 Ivan U. Pascal |   e-mail: [EMAIL PROTECTED]
   Administrator of |   Tomsk State University
 University Network |   Tomsk, Russia
--- xc/programs/Xserver/os/WaitFor.c.orig   Wed Sep 17 17:24:12 2003
+++ xc/programs/Xserver/os/WaitFor.cMon Sep 22 19:34:55 2003
@@ -109,6 +109,7 @@
 CARD32 expires;
 OsTimerCallbackcallback;
 pointerarg;
+intflags;
 };
 
 static void DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev);
@@ -131,17 +132,12 @@
  * pClientsReady is an array to store ready client-index values into.
  */
 
-static INT32 timeTilFrob = 0;  /* while screen saving */
-
 int
 WaitForSomething(int *pClientsReady)
 {
 int i;
 struct timeval waittime, *wt;
 INT32 timeout = 0;
-#ifdef DPMSExtension
-INT32 standbyTimeout = 0, suspendTimeout = 0, offTimeout = 0;
-#endif
 fd_set clientsReadable;
 fd_set clientsWritable;
 int curclient;
@@ -188,138 +184,37 @@
else
{
 #endif
-#ifdef DPMSExtension
-   if (ScreenSaverTime  0 || DPMSEnabled || timers)
-#else
-   if (ScreenSaverTime  0 || timers)
-#endif
-   now = GetTimeInMillis();
-   wt = NULL;
+wt = NULL;
if (timers)
-   {
-   while (timers  (int) (timers-expires - now) = 0)
-   DoTimer(timers, now, timers);
-   if (timers)
-   {
-   timeout = timers-expires - now;
-   waittime.tv_sec = timeout / MILLI_PER_SECOND;
-   waittime.tv_usec = (timeout % MILLI_PER_SECOND) *
-   (100 / MILLI_PER_SECOND);
-   wt = waittime;
-   }
-   }
-   if (ScreenSaverTime  0
-#ifdef DPMSExtension
-   || (DPMSEnabled 
-(DPMSStandbyTime  0 || DPMSSuspendTime  0 || DPMSOffTime  0))
-#endif
-   ) {
-#ifdef DPMSExtension
-   if (ScreenSaverTime  0)
-#endif
-   timeout = (ScreenSaverTime -
-  (now - lastDeviceEventTime.milliseconds));
-#ifdef DPMSExtension
-   if (DPMSStandbyTime  0)
-   standbyTimeout = (DPMSStandbyTime -
- (now - lastDeviceEventTime.milliseconds));
-   if (DPMSSuspendTime  0)
-   suspendTimeout = (DPMSSuspendTime -
- (now - lastDeviceEventTime.milliseconds));
-   if (DPMSOffTime  0)
-   offTimeout = (DPMSOffTime -
- (now - lastDeviceEventTime.milliseconds));
-#endif /* DPMSExtension */
-
-   if (
-   timeout = 0
-#ifdef DPMSExtension
- ScreenSaverTime  0
-#endif /* DPMSExtension */
-   ) {
-   INT32 timeSinceSave;
-
-   timeSinceSave = -timeout;
-   if (timeSinceSave = timeTilFrob  timeTilFrob = 0)
-   {
-   ResetOsBuffers(); /* not ideal, but better than nothing */
-   SaveScreens(SCREEN_SAVER_ON, ScreenSaverActive);
-#ifdef DPMSExtension
-   if (ScreenSaverInterval  0 
-   DPMSPowerLevel == DPMSModeOn)
-#else
-   if (ScreenSaverInterval)
-#endif /* DPMSExtension */
-   /* round up to the next ScreenSaverInterval */
-   timeTilFrob = ScreenSaverInterval *
-   ((timeSinceSave + ScreenSaverInterval) /
-   ScreenSaverInterval);
-   else
-   timeTilFrob = -1;
-   }
-   timeout = timeTilFrob - timeSinceSave;
-   }
-   else
-   {
-   if (ScreenSaverTime  0  timeout  ScreenSaverTime)
-   timeout = ScreenSaverTime;
-   timeTilFrob = 0;
-   }
-#ifdef DPMSExtension
-   if (DPMSEnabled)
-   {
-   if (standbyTimeout  0 
-(timeout = 0 || timeout  

Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-22 Thread David Dawes
On Mon, Sep 22, 2003 at 09:27:53PM +0700, Ivan Pascal wrote:

I made new version of the patch.
The issue Keith pointed is fixed.

Also I added a flag for timers TimerNeedsCheckInput.  The timers without this
flag are processed before the select all others are delaied until the second
timers check.  (The second check doesn't distinguish those timers.)

Did we ever find any timers that need to be processed before the select?

Also I found another issue.  The thing is that the procedure xf86Wakeup that
reads input devices processes only one device in a turn because, as it is said
in comments, it Must break [loop] here because more than one device may share
the same file descriptor.  Now the built-in keyboard driver is called
separately before the loop and all works well.  But if a keyboard is served
by a module driver and there are more than one devices with 'ready for reading'
events there is not guaranty that all input devices events will be read before
the timers checking.

I made a separate patch for xf86Wakeup (in a separate attachment).
Please review.

Both patches look OK to me from a read through.

David
-- 
David Dawes X-Oz Technologies
www.XFree86.org/~dawes  www.x-oz.com
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-21 Thread Egbert Eich
Keith Packard writes:
  Around 20 o'clock on Sep 20, Ivan Pascal wrote:
  
   The solution is obvious.  WaitForSomething should not run any callbacks before
   the Select but just check timers and if there are expired timers call select
   with a zero timeout.  Also the code before the select checks screensaver and
   DPMS timeouts.
  
  One use for timeouts is to handle external network events, such as font 
  server reconnect or XDMCP messages.  In those cases, the timeout routine 
  can modify which sockets will be needed in the select mask, hence the 
  desire for the timeout routine to execute before the select (and various 
  BlockHandler) routines are invoked.  When the timeout routine fires only 
  after select returns, the server will go through a complete scheduling 
  interval before the BlockHandler is invoked.
  

So what would be the way out? I suppose one could use a wakeup handler
instead. This however would only work if the wakeup handler for keyboard 
input was called before the xkb timer handler. The latencies of
other handlers that are called before the xkb handler could also
cause the timer to expire with the key release unnoticed.

The only way to ensure that a possible release event is detected is
to read the keyboard in the xkb callback handler to check for 
pending release events. However I'm afraid xkb simply doesn't work 
that way.

Another solution may be a a SIGIO driven keyboard input.

   Also the code before the select checks screensaver and DPMS timeouts.  I
   don't see reason why those timeouts are processed separately from timers
   and don't use timers facility.
  
  ScreenSavers used to be the only timeout and were coded before the general 
  timeout mechanism was added; DPMS was kludged in on top of that mechanism. 
  When the general timeout mechanism was added, the screen saver/DPMS code 
  should have been migrated over.
  

Yes, Ivan's patch fixed that.

Egbert.
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-21 Thread Harold L Hunt II
David,

David Dawes wrote:

On Sun, Sep 21, 2003 at 12:13:10AM -0400, Harold L Hunt II wrote:


The only suggestion I have is that the function prototypes in 
include/os.h should follow the conventions of all other prototypes in 
os.h, using #if NeedFunctionPrototypes:

extern void SetScreenSaverTimer(
#if NeedFunctionPrototypes
void
#endif
);
#ifdef DPMSExtension
extern void SetDPMSTimers(
#if NeedFunctionPrototypes
void
#endif
);
#endif


Actually, we're going the other way and on the trunk all of the
NeedFunctionPrototypes references have been removed in os.h.
I missed that.  Sorry for the noise.


Thanks for taking the time to debug this properly... I hope that there 


Yes!  Hopefully the screen saver/DPMS cleanup will reduce the likelihood
of further subtle bugs that have plagued that code for a while.
Cleanup is good.

Harold

___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel


Re: Wrong order of a timeouts processing in WaitForSomething.

2003-09-21 Thread Keith Packard
Around 21 o'clock on Sep 21, Egbert Eich wrote:

 That was my suggestion. But I'm afraid xkb doesn't work like that.
 Not xkb checks for pending input the ddx layer does in a wakeup 
 handler that does so and passes information on to xkb. 
 The timeout handling would have to be moved from xkb to ddx.

The XKB code could call ProcessInputEvents just above the check for a 
pending repeat key; that would ensure that any input queued either in the 
X server or the kernel would get processed before repeat status was 
checked.

 I agree, but I don't think there is a restriction on the amount of 
 processing done before the events are queued. Therefore events could
 be processed far enough to know if a pending timer for a repeat should
 be canelled. But then this would require more extensive changes in xkb.

The server really can't process events even this far -- detecting which 
keysyms are associated with each key requires accessing keymap tables 
which may be manipulated by clients outside of the signal handler.  It's 
far easier to restrict SIGIO processing to event queuing -- that requires 
no global server state aside from the event queue itself.

 I think that's what Ivan's patch does: The only place where timers are
 called is after the select and the WakeupHandlers.

The timers are invoked just before the test for queued input; moving the 
queued input test above that should eliminate cases where key events are 
queued with SIGIO and then not processed before the timer function is 
invoked.   In any case, the current spot isn't sufficient as the timers 
will never be executed if the server is constantly receiving application 
input, so at least that needs to be fixed.

-keith


___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel