[kde-freebsd] QFilesSystemWatcher instead of polling/FAM
Tijl Coosemans
tijl at coosemans.org
Fri Aug 6 14:39:29 CEST 2010
On Tuesday 03 August 2010 20:21:48 Max Brazhnikov wrote:
> On Mon, 2 Aug 2010 22:10:52 +0200, Tijl Coosemans wrote:
>> On Monday 19 July 2010 14:07:56 Rusty Nejdl wrote:
>>> On Mon, 19 Jul 2010 12:20:35 +0200, Jaap Boender wrote:
>>>>>>>> I've attached a patch that makes KDE use QFileSystemWatcher on
>>>>>>>> FreeBSD which uses kqueue instead of stat. I've been using it for
>>>>>>>> the past few days and it seems to work rather well.
>>>>
>>>> I've been using the patch (the July 8 version), and it works well
>>>> for the most part; the only problem is that plasma-desktop runs up
>>>> to 100% CPU usage every once in a while (and becomes unresponsive),
>>>> so that I've got to kill and restart.
>>>>
>>>> Looking at the logs, there doesn't seem to be any particular error
>>>> message, and it doesn't seem to be triggered by any specific action.
>>>> If you need me to do anything specific for debugging purposes, or
>>>> need more information, let me know.
>>>
>>> I can completely attest to this and mine was so bad I had to remove
>>> the patch. Plasma would die but everything else would work after
>>> that.
>>
>> It turns out that Qt uses a continue statement in a do-while loop
>> thereby creating an infinite loop. The attached patch should solve it.
>> You have to put it in devel/qt4-corelib/files/ and rebuild that port.
>>
>> I'm still trying to fix some more issues, but this should already
>> help a lot, so please give it a try.
>
> Still crashes with this patch and your latest patch for kdelibs:
Here's an updated patch for devel/qt4-corelib. It turns out that kded4
monitors a lot of files (2000+ in my case). That means the process
(such as other parts of Qt) can no longer use select(2) on descriptors
opened after those 2000+ because that's limited to 1024 (FD_SETSIZE).
Qt doesn't do a bounds check when it tries to use such a descriptor
with select and essentially a buffer overflow occurs that can cause
a crash later on.
This patch includes a number of other changes. Below are the commit
logs for those interested.
I'm also still working on a more extensive patch for kdirwatch.
QKqueueFileSystemWatcherEngine: Use EV_CLEAR instead of EV_ONESHOT.
Using EV_ONESHOT and re-enabling the kevent after emitting the signal
allows for a window in which file system changes can go undetected. By
using EV_CLEAR instead the kevent can stay enabled.
QKqueueFileSystemWatcherEngine: Deleting kevent is handled by close().
QKqueueFileSystemWatcherEngine: Fix infinite loop cases.
While the application thread is removing the watch on a path, it is
still possible for the worker thread to process events for that path on
another CPU. In the case where the worker thread detects that the event
is no longer valid, there's a continue statement inside a do-while loop
causing an infinite loop, because the next event is never retrieved and
the same event is found invalid over and over.
The do-while loop can simply be removed. This has the added benefit that
the lock is released between processing events, shortening the critical
section and allowing the application thread to add and remove watches
more easily.
QKqueueFileSystemWatcherEngine: Handle kevent(2) returning EINTR.
The worker thread exits whenever the kevent call returns an error, but
in the case of EINTR (interrupted by signal) it should just call kevent
again. Otherwise for instance, attaching a debugger to the process
causes the worker thread to exit because of the SIGSTOP it receives.
QKqueueFileSystemWatcherEngine: Minor tweaks to locking.
- Calls to write(2) potentially block, so make sure the application thread
unlocks the mutex before it writes to the pipe between itself and the
worker thread, so the latter can continue to process events and eventually
unblock the write call (if needed) by emptying the pipe.
- In the worker thread there's no need to lock the mutex when processing a
pipe event. Generally the worker thread should hamper the application
thread as little as possible, so only lock the mutex where needed.
QKqueueFileSystemWatcherEngine: Use higher file descriptors.
A file descriptor is used for every path to be monitored, but descriptors
below FD_SETSIZE (typically 1024) are precious, for use with select(2). To
allow the application (and other parts of Qt) to use select(2), try to
duplicate the descriptor returned by open(2) above FD_SETSIZE and close(2)
the original. However, only do so when the descriptor table is already
fairly large (FD_SETSIZE / 2). This keeps the descriptor table small for
applications that use only a few descriptors.
While here, also set the close-on-exec flag on the (new) descriptor.
-------------- next part --------------
diff --git src/corelib/io/qfilesystemwatcher_kqueue.cpp src/corelib/io/qfilesystemwatcher_kqueue.cpp
index 99c165e..ce79cf7 100644
--- src/corelib/io/qfilesystemwatcher_kqueue.cpp
+++ src/corelib/io/qfilesystemwatcher_kqueue.cpp
@@ -133,6 +133,14 @@ QStringList QKqueueFileSystemWatcherEngine::addPaths(const QStringList &paths,
perror("QKqueueFileSystemWatcherEngine::addPaths: open");
continue;
}
+ if (fd >= FD_SETSIZE / 2 && fd < FD_SETSIZE) {
+ int fddup = fcntl(fd, F_DUPFD, FD_SETSIZE);
+ if (fddup != -1) {
+ ::close(fd);
+ fd = fddup;
+ }
+ }
+ fcntl(fd, F_SETFD, FD_CLOEXEC);
QT_STATBUF st;
if (QT_FSTAT(fd, &st) == -1) {
@@ -157,7 +165,7 @@ QStringList QKqueueFileSystemWatcherEngine::addPaths(const QStringList &paths,
EV_SET(&kev,
fd,
EVFILT_VNODE,
- EV_ADD | EV_ENABLE | EV_ONESHOT,
+ EV_ADD | EV_ENABLE | EV_CLEAR,
NOTE_DELETE | NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_RENAME | NOTE_REVOKE,
0,
0);
@@ -180,6 +188,8 @@ QStringList QKqueueFileSystemWatcherEngine::addPaths(const QStringList &paths,
idToPath.insert(id, path);
}
+ locker.unlock();
+
if (!isRunning())
start();
else
@@ -203,19 +213,7 @@ QStringList QKqueueFileSystemWatcherEngine::removePaths(const QStringList &paths
if (x.isEmpty() || x != path)
continue;
- int fd = id < 0 ? -id : id;
- struct kevent kev;
- EV_SET(&kev,
- fd,
- EVFILT_VNODE,
- EV_DELETE,
- NOTE_DELETE | NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_RENAME | NOTE_REVOKE,
- 0,
- 0);
- if (kevent(kqfd, &kev, 1, 0, 0, 0) == -1) {
- perror("QKqueueFileSystemWatcherEngine::removeWatch: kevent");
- }
- ::close(fd);
+ ::close(id < 0 ? -id : id);
it.remove();
if (id < 0)
@@ -225,11 +223,11 @@ QStringList QKqueueFileSystemWatcherEngine::removePaths(const QStringList &paths
}
if (pathToID.isEmpty()) {
+ locker.unlock();
stop();
- locker.unlock();
wait();
- locker.relock();
} else {
+ locker.unlock();
write(kqpipe[1], "@", 1);
}
@@ -243,19 +241,18 @@ void QKqueueFileSystemWatcherEngine::stop()
void QKqueueFileSystemWatcherEngine::run()
{
- static const struct timespec ZeroTimeout = { 0, 0 };
-
forever {
struct kevent kev;
DEBUG() << "QKqueueFileSystemWatcherEngine: waiting for kevents...";
int r = kevent(kqfd, 0, 0, &kev, 1, 0);
if (r < 0) {
+ if(errno == EINTR) {
+ DEBUG() << "QKqueueFileSystemWatcherEngine: kevent call was interrupted, restarting...";
+ continue;
+ }
perror("QKqueueFileSystemWatcherEngine: error during kevent wait");
return;
- }
-
- QMutexLocker locker(&mutex);
- do {
+ } else {
int fd = kev.ident;
DEBUG() << "QKqueueFileSystemWatcherEngine: processing kevent" << kev.ident << kev.filter;
@@ -287,6 +284,8 @@ void QKqueueFileSystemWatcherEngine::run()
break;
}
} else {
+ QMutexLocker locker(&mutex);
+
int id = fd;
QString path = idToPath.value(id);
if (path.isEmpty()) {
@@ -315,30 +314,15 @@ void QKqueueFileSystemWatcherEngine::run()
else
emit fileChanged(path, true);
} else {
- DEBUG() << path << "changed, re-enabling watch";
+ DEBUG() << path << "changed";
if (id < 0)
emit directoryChanged(path, false);
else
emit fileChanged(path, false);
-
- // renable the watch
- EV_SET(&kev,
- fd,
- EVFILT_VNODE,
- EV_ADD | EV_ENABLE | EV_ONESHOT,
- NOTE_DELETE | NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_RENAME | NOTE_REVOKE,
- 0,
- 0);
- if (kevent(kqfd, &kev, 1, 0, 0, 0) == -1) {
- perror("QKqueueFileSystemWatcherEngine::processKqueueEvents: kevent EV_ADD");
- }
}
}
-
- // are there any more?
- r = kevent(kqfd, 0, 0, &kev, 1, &ZeroTimeout);
- } while (r > 0);
+ }
}
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 228 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/kde-freebsd/attachments/20100806/c184516a/attachment.sig
More information about the kde-freebsd
mailing list