On Wed, 2008-06-18 at 09:02 +0200, Thomas Hellström wrote: > Keith Packard wrote: > > On Mon, 2008-06-16 at 10:24 +0200, Thomas Hellström wrote: > > > > > >> IMHO this seems like a very odd commit. > >> > > > > We're now using libdrm for all X server device interaction, which means > > having every return deal correctly with signals (as the X server > > receives a lot of signals). What we want is restartable ioctls, but > > POSIX doesn't allow those. > > > > Are there DRM functions which can be interrupted and which shouldn't be > > restarted? It seems crazy to me. However, it's likely that we should > > only restart on EINTR and not on EAGAIN. EINTR should only be returned > > when interrupted by a signal, and restarting makes the ioctls look like > > other 'sane' syscalls like read and write. > > > > > My main concern is with ioctls that aren't interruptible, like > DRM_IOCTL_VERSION. > Why should we have a check on those, when they never return -EAGAIN or > -EINTR? > That looks bad and will confuse people (new drm developers) to think > restarting is always needed.
That would require auditing forwards and backwards to ensure that signals are handled correctly in all existing and new ioctls. Creating a pattern where every ioctl always loops on EINTR means that new functions which do receive signals will 'just work' instead of depending on the developer to carefully analyse their code. We're just trying to recover from broken POSIX ioctl semantics in my view. Given that libdrm is now used extensively within a signal-rich environment (the X server), I'd say masking POSIX behaviour within our library is a sensible plan. > On a side note, doesn't read and write return -EINTR on signals? No. read and write restart internally on > Any dri app that uses a lot of signals will probably generate a lot of > error messages that aren't really errors > and that will confuse users. Not that I know of many such apps, but this > will probably cause that error > printout to be removed sooner or later. Sure, I plan on removing it as soon as the GEM drivers are stable enough; it has been rather useful for debugging at present though :-) > Converting > to mutex_lock_interruptible would be made easier if there was a > dev->gem_mutex. This is, of course, assuming > that the mutex _is_ indeed held around gpu waits. Yes, the mutex is held across execbuffer execution to avoid live-locking problems. Finer grained locking is always possible, but introduces additional locking cost and lock-order issues. -- [EMAIL PROTECTED]
signature.asc
Description: This is a digitally signed message part
------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php
-- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
