Bug 154676

Yes, this bug is still in my box (Dolphin 1.0.99; KDE 4.00.64 >= 20080228 "release 2.2"). Way to reproduce:

1) Browse your home folder with dolphin.
2) Open a Konsole in your home folder, execute "echo 123 > 123". File 123 will appear in dolphin.
3) execute "chmond +x 123" in Konsole. Then browse the permissions in dolphin, you'll see the data is out of date.
4) execute "echo 1234 > 123". Then everything in dolphin goes right.

(I've added David Faure to CC)

> This used to work in KDE 3.X.

I've just verified this with KDE 3 and changing the permissions does not lead to an update in KDE 3. For sure I agree that it would be nice if the update would be reflected without the need of reloading, but AFAIK KDE depends here on the Kernel/filesystem configuration. @David: please correct me if I'm telling nonsense here ;-)

No idea of inotify supports this.
However we could use KDirNotify to force a refresh (== kde-level DBUS notification).

"I've just verified this with KDE 3 and changing the permissions does not lead to an update in KDE 3."

Actually, it does - but you have to go back a long, long way to see it in action ;) I've just verified this in Mandrake 10.0 (KDE 3.2) in a VM, and externally made changes to files caused automatic refreshes of the Konqueror view, with fam being responsible for the detection/ reporting.  The feature stopped working some time before 3.4.

I'm planning to have a look into this wishlist item myself as it is the KDE4 version of my Pet Bug ( http://bugs.kde.org/show_bug.cgi?id=106394 ) but, as David notes, it all depends on whether inotify reports this kind of file attribute modification.

Well, that was easy - looks like all of the infrastructure was already in place, and just needed a small nudge!  A tiny patch follows.

With the patch, and an inotify-enabled system, external changes made to a file - chmod's, touch'ing, echo'ing, etc - are shown in any Dolphin/ Konqueror panes that are open at the directory containing the file.  

The "refreshing" is lightweight: the other files in the directory aren't re-thumbnailed/ mime-type'd as would be the case with a "manual" refresh, so CPU usage is low.  Change events seemed to be dispatched every 2 seconds or so, even if the file is altered much more frequently.  The patch does not appear to introduce instability in Dolphin or Konqueror or in a running KDE session.  I'm not sure if any additional tests need be added to kdirlistertest.cpp ... ?

Created attachment 26183 [details]
Patch to kdirlister_p.h that fixes this bug

Very interesting :)
This looks good, obviously we want file managers to be told of file changes.
I'm just worried that this is going to increase greatly the number of things watched
by KDirWatch, which could hit inotify or fam limits (and create a huge number of
threads on Windows, since there the limit is 64 files watched per thread iirc).

For what's it's worth, in KDE 3 changes to a single file where not reported either,
at least not with Stat for sure (it was only looking at the mtime of the directory,
so it would only detect new files or deleted files).
I thought it worked with FAM, but I'm not sure (I'm no KDirWatch expert).

Flavio? Do you see a problem with KDirLister (i.e. dolphin, konqueror, the file dialog, etc.)
watching all files?  (This isn't only about files currently visible in the GUI, but also
those that have been previously listed and that are now in KDirLister's cache)

Hi David,

I'm no expert, but I'm reasonably sure that monitoring a directory with inotify - as is already done, even without this patch - gives you individual file changes "for free" - it's just that they KDirLister did not ask KDirWatch for them before i.e. KDirWatch was receiving these reports, but not forwarding them on to KDirLister.  I could very well be wrong, though, and I don't know anything about other platform's notification systems.

I'll forward this on to kde-devel and see if anyone else has some insight into all this file access monitoring stuff :) 

Ok, it looks like inotify_add_watch performed on a directory will report changes to all the files within that directory as long as IN_MODIFY and IN_ATTRIB are in the watch masks - I don't know if this comes at a price, however, and can't find any definitive info on the topic.

Currently, though, KDirWatch will add a watch to each individual file in a directory if WatchFiles is used, which almost certainly does have a price and does indeed render this patch risky.  It's going to take some fairly heavy re-working to get KDirWatch to not do this.  I'll take a look this weekend - I like a challenge! ;)

@SSJ:
> I'll take a look this weekend - I like a challenge! ;) 

That's great to hear, thanks!

I've just read the whole thread, so here my 2 cents...

Talking about inotify: when you add a watch on a directory you get a notify when something happens inside it (according the the event mask you provide). But you don't know what _exactly_ happened. For example, you don't know if somebody modified a file, created a new directory, deleted a file,... you don't know nor the target nor the action of the event.
In order to know what _exactly_ occurred you have to add a watch for each file in the directory . And this is what SSJ's patch is doing.

Talking about performances: there won't be problems.
Talking about scalability: there will be huge problems due to the limitations of inotify.

In order to understand the scalability problems let me explain how inotify works.

When you want to monitor a resource (that can be a directory or a file) you've to create a watch object. As I said in the beginning, if you want to get accurate informations you've to create a new watch for each file / directory under the path you're going to monitor. Programs like Beagle, Strigi, Nepomuk have to add watches _recursively_.  As you can expect this leads to the creation of tons of watch objects.

Each inotify watch object is allocated using precious kernel's memory. So the kernel hackers decided to limit the number of watches to 8192 watches per user. This limit can be changed by root using procfs (echo XYZ > /proc/sys/fs/inotify/max_user_watches).

Each program run by the user has to face this constraint: this is the scalability problem I was talking about.
If you are running at the same time some programs using inotify (like Beagle, Strigi, Nepomuk and dolphin), you'll probably allocate all the 8192 allowed watches.

This scalability problem is well-known, but there isn't actually a solution.

The only workaround is to use a monitoring facility that shares the same watch between different programs. This is was FAMD should do and this is also what KDirWatch performs.

I would like to start a discussion with kernel hackers and the FLOSS community in order to find a good solution to this problem.
I think that:
- Linux kernel hackers should provide better low-level facilities (and we should tell them what we would like to have)
- FLOSS community should create a good replacement of FAMD. FAMD simply sucks, while Gamin is a bit better but suffers the same limitations of FAMD (since by design decisions it provides the same API and ABI)

In my dreams there's a good low-level facility exposed by a system daemon. All programs that need to monitor file system will use this new system daemon.

BTW: right by now I've just talked about the Linux scene. Let me give you a short overview:
- free *BSD platforms: compared to Linux the situation is worse, since inotify is infinitely better than kqueue.
- macosx: there should be a good low-level facility, but I have not been able to play with it (lack of time).
- win2k based systems: there is something for ntfs partitions, but I've never studied it (lack of time).

Forgive me for the long reply. If you want you can find a thread with a similar topic on kde-code-devel ml (just search my name in the archives).

Hi Flavio, 

Thanks very much for taking the time to write such a detailed reply. I am, however, still confused :s Here are the assumptions I'm working under; please point out where I am going wrong.

1. Dolphin is open on a directory; "/home/simon/inotify-test", say.

2. To fix this bug, we want to be notified when a file is created, deleted or modified (contents or metadata), and we want to know precisely which file has been altered.

3. We don't care about changes to the contents of subdirectories of "/home/simon/inotify-test"

4. If correctly implemented, KDirWatch with WatchFiles mode will do this: it emits created, deleted and dirty events, with the latter referring to changes to files within "/home/simon/inotify-test".  Actually, now that I come to look at the API - do we need to specify WatchFiles in order for changes to individual files to be watched? It's a little ambiguous.

5. A single inotify_add_watch call on "/home/simon/inotify-test" will report not only deletions and creations of files within "/home/simon/inotify-test", but also changes to individual files themselves, if the mask is IN_MODIFY | IN_CREATE | IN_DELETE | IN_ATTRIB

6. The primary limit to scalability is the 8192 watches per instance.  

7. Adding a watch to "/home/simon/inotify-test" counts as just one watch, whether there are 10, 100, or 100000 files in "/home/simon/inotify-test". This is critical - it seems a little too good to be true, but I've not seen anyone mention that it is false.  Does anyone know for sure? Maybe I should ask Robert Love himself :)

8. Events reported by the watch added to "/home/simon/inotify-test" can be mapped to the particular file in "/home/simon/inotify-test" that was altered - this seems to be borne out by the inotify docs and by a simple test program I have here.  For example, even if "/home/simon/inotify-test" has 100000 files, "chmod ugo-x /home/simon/inotify-test/pickle" will generate an event that tells us that /home/simon/inotify-test/pickle (this file explicity) had its attributes changed, and we don't have to manually watch or check up on any of the 100000 files to determine that it was pickle that changed.

9. The bug can therefore be fixed, for inotify at least, using just one watch per directory.  KDirWatch will have to be re-worked a bit, though, but it should be doable. 

Where am I going wrong?

Forgive me for the delay but I've been out of home this week-end.

I'll give you an answer for each point:
1 - 3) yes, I understand what you're going to do

4) 
> it emits created, deleted and dirty events, with the latter referring to
> changes to files within "/home/simon/inotify-test".
Yes, this is right

> Actually, now that I come to look at the API - do we need to specify 
> WatchFiles in order for changes to individual files to be watched?
Are you talking about the explicit monitoring of a file? I think that in this case you should not have to specify WatchFiles

5) I've run a test program and I've discovered this is true. This is a good news because some times ago it didn't provide the name of the file modified/deleted. Probably somebody fixed it!

6)yes, there's a 8192 watches limit per user. Suppose you're running Strigi (which has taken 4000 watches), Beagle (which has taken 4000 watches too) you can allocate just 192 watches with Dolphin.

7-8) Yes, this is true. As reported in point #5

9) Yes, I think it won't be hard

If you need some help sent me a mail

Hi Flavio,

"Forgive me for the delay but I've been out of home this week-end."

No problem at all - the help is appreciated :)

Your responses are very encouraging (especially 5) and 7)), so I'll look into implementing this in KDirWatch as soon as I can set aside a solid chunk of time.  It's a pretty touch piece of code, though, and I can't see a simple and efficient way of exploiting the advantages of inotify, so I'll probably contact you for hints at some point :)

This is wonderful news! You KDE developers are amazing! Thanks a lot!

*** Bug 167250 has been marked as a duplicate of this bug. ***

A candidate patch has been written, but Flavio is very busy at the moment so has not had time to review it, so I thought I'd post it here in case anyone wants to try it out :) Since I'm pretty new to kde development, there's bound to be a lot of mistakes in there, but hopefully it or something like it will end up being committed :)

A few notes on the implementation:

- KDirWatch seems to rely on the creation of Entry's to report file changes (one per changed file path) at each scan, but this is not really desirable in the case of inotify as each Entry consumes a precious inotify watch.  For this reason, I instead log a list of files that have changed since the last scan.  This works well, avoids gobbling inotify watches and probably uses a smidgeon less memory that the Entry system, but kind of "sticks out" a lot.

- A mutex is used to serialise access to this list, which might scare some people :) I don't see any scope for deadlocking/ other threading problems caused by this, though.

- It fixes this bug only if you use inotify as the kdirwatch backend.

Created attachment 26948 [details]
Make proper use of inotify in kdirwatch.

Should have read the docs :) Everything that alters/ reads m_pendingFileChanges occurs in the same thread, so the mutex is completely unnecessary.  I'll work on a much simpler patch tomorrow evening.

Created attachment 27090 [details]
Updated and simplified patch with no mutex

Hi Simon, 
first of all shame on me for the delay!

I have reviewed your patch and it seems good.
I've applied it, rebuild the necessary code and then I've started Dolphin. It seems that the bug is not fixed. Is it necessary to patch Dolphin?

Cheers
Flavio

Hi Flavio,

No problem about the delay; glad the patch is acceptable. Thanks for reviewing it!

The patch should "just work", as KDirLister etc are already dirty()-aware.  Do you have any simple dummy apps that create a KDirWatch on a folder and listen out for "dirty()" events that you can run quickly? This seems like the quickest way of finding out what is wrong.  Is inotify the default system used by KDirWatch on your system? Has kded been restarted? I'm not sure if the latter is necessary, but it's a good idea to check :)

Maybe as a debug aid, you could add a kDebug(e->path+'/'+path) after "// A file in this directory has been changed." if you have time?

I'll investigate asap

Yes there is a simple test app for KDirWatch, see kdelibs/kio/tests/kdirwatchtest.cpp and kdirwatchtest_gui.cpp. Use "make kdirwatchtest" and "make kdirwatchtest_gui" to compile them, and run them from the builddir.

*** Bug 172854 has been marked as a duplicate of this bug. ***

Sorry to bump this. But it seems to me as if this bug has been resolved since August, but the patch not committed since it was never properly reviewed. Has it been forgotten?

Thanks for your great work!

mutlu

Not forgotten, but I personally think it's a little risky to add it for 4.2, now (this was one of my "newbie" patches, and messing around with KDirWatch can have rather severe consequences as it is used by kded4 :)).

The ideal from my point of view would be to commit it right at the very beginning of the 4.3 cycle.  If anyone else judges that it can go in now, though, then be my guest :)

*** Bug 144379 has been marked as a duplicate of this bug. ***

Any objections to committing this now that trunk is open? I'd like to get it in early in a cycle for safety :)

No objections from my side :-)

It would be great

CCMAIL != CCBUG (whoops).  I won't mark this as FIXED myself as there are other KDirWatch backends e.g. fam.

----
SVN commit 907250 by sstjames


http://websvn.kde.org/?view=rev&revision=907250

Make proper use of inotify's implicit "WatchFiles" ability.  Immediate effect: Dolphin etc should now report changes to file attributes, rather than just creations and deletions, when KDirWatch uses inotify as its backend.  Secondary effect: Assuming no bugs, we should consume at most the same number of inotify watches as before (and often much less).

Approved by Flavio Castelli.

WebSVN link: http://websvn.kde.org/?view=rev&revision=907250

First of all, a big thank you to all involved. This is superb! :)

About closing the bug: I would be fine with closing it now that the functionality is provided by KDirWatch + inotify. Not all functionality can be offered by every single Gnu/Linux tool. However, if someone intends to have a go at fam support, please comment so I leave this open.

And, again, thank you!

Closing, then, if the reporter says we can close it and it doesn't look like anyone is going to work more on the issue :)

SVN commit 957379 by dfaure:

Backport r907250 by sstjames:
"Make proper use of inotify's implicit "WatchFiles" ability.  Immediate effect: Dolphin etc should now report changes to file attributes, rather than just creations and deletions, when KDirWatch uses inotify as its backend.  Secondary effect: Assuming no bugs, we should consume at most the same number of inotify watches as before (and often much less)."
CCBUG: 154676

As a third effect, it fixes kdirlistertest being stuck in KDirListerTest::testRenameAndOverwrite() because
kdirwatch wasn't informing kdirlister of the newly recreated file.


 M  +63 -10    kdirwatch.cpp  
 M  +9 -3      kdirwatch_p.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=957379