After conversations with Andrew, improved the robustness of the

sharemode db in the following way.

Originally, on startup and shutdown, smbd would scan the share mode
db to ensure it was correct. This lead to scalability issues as
scans lock the db for quite a long time. Andrew had the brainstorm
that we only care about the record we're about to read.

This new code (small change really, but quite significant) causes
get_share_modes() to do a process_exists() call against each pid
in each record, and to delete any that don't and re-write the
entry if any dead records were detected.

This allowed me to remove the startup/shutdown scans of the
db (they can be added into smbstatus if anyone really cares to
have them back). This will please the vfs author who was worried
about the time taken on open() calls, and will lead to much
greater robustness and scalability in the share mode db.

We need much testing of this, and also netbench tests to
ensure the extra process_exists() calls don't hurt performance
(they shouldn't it's a very simple system call).

Jeremy.

1 files changed, 53 insertions, 26 deletions

diff --git a/source/locking/locking.c b/source/locking/locking.c
index 023ec59e892..8a8a33b808d 100644
--- a/source/locking/locking.c
+++ b/source/locking/locking.c
@@ -275,8 +275,6 @@ static int open_read_only;
 
 BOOL locking_init(int read_only)
 {
-	BOOL check_self = False;
-
 	brl_init(read_only);
 
 	if (tdb)
@@ -295,10 +293,6 @@ BOOL locking_init(int read_only)
 	if (!posix_locking_init(read_only))
 		return False;
 
-	/* delete any dead locks */
-	if (!read_only)
-		tdb_traverse(tdb, delete_fn, &check_self);
-
 	open_read_only = read_only;
 
 	return True;
@@ -310,20 +304,10 @@ BOOL locking_init(int read_only)
 
 BOOL locking_end(void)
 {
-#if 0
-	BOOL check_self = True;
-#endif
 
 	brl_shutdown(open_read_only);
 	if (tdb) {
 
-		/* delete any dead locks */
-
-#if 0 /* Don't scan on close. */
-		if (!open_read_only)
-			tdb_traverse(tdb, delete_fn, &check_self);
-#endif
-
 		if (tdb_close(tdb) != 0)
 			return False;
 	}
@@ -397,28 +381,71 @@ void unlock_share_entry_fsp(files_struct *fsp)
 
 int get_share_modes(connection_struct *conn, 
 		    SMB_DEV_T dev, SMB_INO_T inode, 
-		    share_mode_entry **shares)
+		    share_mode_entry **pp_shares)
 {
 	TDB_DATA dbuf;
 	struct locking_data *data;
-	int ret;
+	int num_share_modes;
+	share_mode_entry *shares = NULL;
 
-	*shares = NULL;
+	*pp_shares = NULL;
 
 	dbuf = tdb_fetch(tdb, locking_key(dev, inode));
 	if (!dbuf.dptr)
 		return 0;
 
 	data = (struct locking_data *)dbuf.dptr;
-	ret = data->u.num_share_mode_entries;
-	if(ret)
-		*shares = (share_mode_entry *)memdup(dbuf.dptr + sizeof(*data), ret * sizeof(**shares));
-	SAFE_FREE(dbuf.dptr);
+	num_share_modes = data->u.num_share_mode_entries;
+	if(num_share_modes) {
+		int i;
+		int del_count = 0;
 
-	if (! *shares)
-		return 0;
+		shares = (share_mode_entry *)memdup(dbuf.dptr + sizeof(*data),	
+								num_share_modes * sizeof(share_mode_entry));
 
-	return ret;
+		if (!shares) {
+			SAFE_FREE(dbuf.dptr);
+			return 0;
+		}
+
+		/*
+		 * Ensure that each entry has a real process attached.
+		 */
+
+		for (i = 0; i < num_share_modes; ) {
+			share_mode_entry *entry_p = &shares[i];
+			if (process_exists(entry_p->pid))
+				i++;
+			else {
+				memcpy( &shares[i], &shares[i+1],
+							sizeof(share_mode_entry) * (num_share_modes - i - 1));
+				num_share_modes--;
+				del_count++;
+			}
+		}
+
+		/* Did we delete any ? If so, re-store in tdb. */
+		if (del_count) {
+			data->u.num_share_mode_entries = num_share_modes;
+			
+			if (num_share_modes)
+				memcpy(dbuf.dptr + sizeof(*data), shares,
+						num_share_modes * sizeof(share_mode_entry));
+
+			/* The record has shrunk a bit */
+			dbuf.dsize -= del_count * sizeof(share_mode_entry);
+
+			if (tdb_store(tdb, locking_key(dev, inode), dbuf, TDB_REPLACE) == -1) {
+				SAFE_FREE(shares);
+				SAFE_FREE(dbuf.dptr);
+				return 0;
+			}
+		}
+	}
+
+	SAFE_FREE(dbuf.dptr);
+	*pp_shares = shares;
+	return num_share_modes;
 }
 
 /*******************************************************************