Bug 2285 - shortcut common cases for FindFirstChangeNotification calls
Summary: shortcut common cases for FindFirstChangeNotification calls
Status: CLOSED FIXED
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: File Services (show other bugs)
Version: 3.0.11
Hardware: SGI IRIX
: P3 normal
Target Milestone: none
Assignee: Samba Bugzilla Account
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-01-27 23:39 UTC by James Peach
Modified: 2005-08-24 10:16 UTC (History)
0 users

See Also:


Attachments
shortcut notification hash generation (2.89 KB, patch)
2005-01-27 23:46 UTC, James Peach
no flags Details
shortcut notification hash generation (4.64 KB, patch)
2005-06-07 00:38 UTC, James Peach
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description James Peach 2005-01-27 23:39:04 UTC
The path in smbd/notify_hash.c is quite slow. Any non-linux Samba server using
this code path will end up doing a *lot* more readdirs in response to Windows
Explorer file management operations.

I'm reporting this as SGI/IRIX, but it really applies to every platform that
doesn't have the Linux-style F_NOTIFY fcntl.
Comment 1 James Peach 2005-01-27 23:46:30 UTC
Created attachment 920 [details]
shortcut notification hash generation

This patch shortcuts a common case where the client asks to watch for changes
to file or directory names in a directory. We can tell when these change by
looking at the directory mtime (which is updated when a file or directory is
created or removed) and the number of links in the directory (which is updated
based on the number of subdirectories).
Comment 2 Jeremy Allison 2005-02-03 15:09:47 UTC
The submitted patch only works on systems with a microsecond timestamp
resolution in stat (ie. the stat struct has st_mtim struct timespec elements).
We can't depend on this (most systems don't have this yet).
It needs to be made feature specific (configure tests etc.).
Jeremy.
Comment 3 Jeremy Allison 2005-02-04 16:16:26 UTC
I must be being dumb, but I don't understand this part of the patch.
From my understanding st_nlink is the number of subdirectories
within a directory (counting . and ..). So what is this doing exactly ?

Jeremy.

+        /* If we are only looking at some combination of CHANGE_NAME and
+         * CHANGE_DIR_NAME, then we can determine this solely from the
+         * information in the directory entry.
+         *      the {c,m}time tells us whether something was changed
+         *      the nlinks field tells us whether a directory entry was changed.
+         */
+        if (S_ISDIR(st.st_mode) && 
+            (flags & ~(FILE_NOTIFY_CHANGE_FILE | FILE_NOTIFY_CHANGE_DIR_NAME))
== 0)
+        {
+                data->num_entries = st.st_nlink;
+                return True;
+        }
Comment 4 James Peach 2005-02-06 15:42:12 UTC
(In reply to comment #3)
> I must be being dumb, but I don't understand this part of the patch.
> From my understanding st_nlink is the number of subdirectories
> within a directory (counting . and ..). So what is this doing exactly ?

No, you're right, stashing st_nlink is redundant. The important thing is that
if the caller only cares about modifications to the directory contents, this
will be reflected in the mtime and we don't need to generate the hash.
Comment 5 Gerald (Jerry) Carter (dead mail address) 2005-02-07 07:29:27 UTC
resetting version
Comment 6 James Peach 2005-06-07 00:38:12 UTC
Created attachment 1263 [details]
shortcut notification hash generation

This patch removes the unnecessary use on nlinks, adds configure checks for
high resolution timestamps in struct stat and clarifies a comment.
Comment 7 Jeremy Allison 2005-07-21 18:14:09 UTC
Applied, thanks ! For the last time - you can do your own now :-).
Jeremy.
Comment 8 Gerald (Jerry) Carter (dead mail address) 2005-08-24 10:16:53 UTC
sorry for the same, cleaning up the database to prevent unecessary reopens of bugs.