[PATCH]: NetSessEnum - Removing a loop around locking tdb traversal
Jeremy Allison
jra at samba.org
Thu Jan 16 12:36:01 MST 2014
On Fri, Jan 10, 2014 at 11:13:47AM -0700, Christof Schmitt wrote:
>
> Thanks for finding that and explaining the problem, i missed it in my
> review. I will wait for the corrected patchset.
Shekhar, Christof,
As promised, here is the corrected patchset. It's mostly
just Shekhar's patchset with an initial change that
removes the problem.
It compiles :-) but still needs testing :-).
Can you test and give me feedback ?
Thanks,
Jeremy.
-------------- next part --------------
From 78614c7d753f17b44c1d6a7fcafceb18da63d97b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 16 Jan 2014 10:43:30 -0800
Subject: [PATCH 1/3] s3: rpc_server/srvsvc: Ensure we don't continually
realloc inside init_srv_sess_info_1().
Just allocate the return value directly. Makes iteration of open files much easier.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 33 ++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index 6707805..bb106ca 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -862,7 +862,25 @@ static WERROR init_srv_sess_info_1(struct pipes_struct *p,
*total_entries = list_sessions(p->mem_ctx, &session_list);
- for (; resume_handle < *total_entries; resume_handle++) {
+ if (resume_handle >= *total_entries) {
+ if (resume_handle_p) {
+ *resume_handle_p = 0;
+ }
+ return WERR_OK;
+ }
+
+ /* We know num_entries must be positive, due to
+ the check resume_handle >= *total_entries above. */
+
+ num_entries = *total_entries - resume_handle;
+
+ ctr1->array = talloc_zero_array(p->mem_ctx,
+ struct srvsvc_NetSessInfo1,
+ num_entries);
+
+ W_ERROR_HAVE_NO_MEMORY(ctr1->array);
+
+ for (num_entries = 0; resume_handle < *total_entries; num_entries++, resume_handle++) {
uint32 num_files;
uint32 connect_time;
struct passwd *pw = getpwnam(session_list[resume_handle].username);
@@ -871,27 +889,20 @@ static WERROR init_srv_sess_info_1(struct pipes_struct *p,
if ( !pw ) {
DEBUG(10,("init_srv_sess_info_1: failed to find owner: %s\n",
session_list[resume_handle].username));
- continue;
+ num_files = 0;
+ } else {
+ num_files = net_count_files(pw->pw_uid, session_list[resume_handle].pid);
}
connect_time = (uint32_t)(now - session_list[resume_handle].connect_start);
- num_files = net_count_files(pw->pw_uid, session_list[resume_handle].pid);
guest = strequal( session_list[resume_handle].username, lp_guestaccount() );
- ctr1->array = talloc_realloc(p->mem_ctx,
- ctr1->array,
- struct srvsvc_NetSessInfo1,
- num_entries+1);
- W_ERROR_HAVE_NO_MEMORY(ctr1->array);
-
ctr1->array[num_entries].client = session_list[resume_handle].remote_machine;
ctr1->array[num_entries].user = session_list[resume_handle].username;
ctr1->array[num_entries].num_open = num_files;
ctr1->array[num_entries].time = connect_time;
ctr1->array[num_entries].idle_time = 0;
ctr1->array[num_entries].user_flags = guest;
-
- num_entries++;
}
ctr1->count = num_entries;
--
1.8.5.2
From 95e61463f248706e0eb0f85a2bc4b0362ecfeae7 Mon Sep 17 00:00:00 2001
From: Shekhar Amlekar <samlekar at in.ibm.com>
Date: Wed, 8 Jan 2014 11:32:21 +0530
Subject: [PATCH 2/3] s3: rpc_server/srvsvc: Adding functions to determine open
files on all sessions.
Introduce helper functions for counting the number of open files on an
array of sessions.
Signed-off-by: Shekhar Amlekar <samlekar at in.ibm.com>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 55 +++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index bb106ca..e901550 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -60,6 +60,13 @@ struct sess_file_count {
int count;
};
+struct sess_file_info {
+ struct srvsvc_NetSessCtr1 *ctr;
+ struct sessionid *session_list;
+ uint32_t resume_handle;
+ uint32_t num_entries;
+};
+
/*******************************************************************
********************************************************************/
@@ -837,6 +844,54 @@ static int net_count_files( uid_t uid, struct server_id pid )
return s_file_cnt.count;
}
+/***********************************************************************
+ * find out the session on which this file is open and bump up its count
+ **********************************************************************/
+
+static void count_sess_files_fn(const struct share_mode_entry *e,
+ const char *sharepath, const char *fname,
+ void *data)
+{
+ struct sess_file_info *info = data;
+ uint32_t rh = info->resume_handle;
+ int i;
+
+ for (i=0; i < info->num_entries; i++) {
+ /* rh+info->num_entries is safe, as we've
+ ensured that:
+ *total_entries > resume_handle &&
+ info->num_entries = *total_entries - resume_handle;
+ inside init_srv_sess_info_1() below.
+ */
+ struct sessionid *sess = &info->session_list[rh + i];
+ if ((e->uid == sess->uid) &&
+ serverid_equal(&e->pid, &sess->pid)) {
+
+ info->ctr->array[i].num_open++;
+ return;
+ }
+ }
+}
+
+/*******************************************************************
+ * count the num of open files on all sessions
+ *******************************************************************/
+
+static void net_count_files_for_all_sess(struct srvsvc_NetSessCtr1 *ctr1,
+ struct sessionid *session_list,
+ uint32_t resume_handle,
+ uint32_t num_entries)
+{
+ struct sess_file_info s_file_info;
+
+ s_file_info.ctr = ctr1;
+ s_file_info.session_list = session_list;
+ s_file_info.resume_handle = resume_handle;
+ s_file_info.num_entries = num_entries;
+
+ share_mode_forall(count_sess_files_fn, &s_file_info);
+}
+
/*******************************************************************
fill in a sess info level 1 structure.
********************************************************************/
--
1.8.5.2
From 2f8d3acb29ce8c285597eba859488ea56af4a034 Mon Sep 17 00:00:00 2001
From: Shekhar Amlekar <samlekar at in.ibm.com>
Date: Thu, 16 Jan 2014 11:10:56 -0800
Subject: [PATCH 3/3] s3: rpc_server/srvsvc: Avoiding the loop around locking
tdb traversal.
The current code for determining the number of open files iterates
over the session list and for each session it traverses the locking
tdb to get the open files. This scales badly for a large server
with many sessions and open files. Instead, get the list of
sessions first, and then determine the number of open files on all
sessions in a single traversal of locking tdb.
Signed-off-by: Shekhar Amlekar <samlekar at in.ibm.com>
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 55 ++++---------------------------
1 file changed, 6 insertions(+), 49 deletions(-)
diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index e901550..f6db6fc 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -54,12 +54,6 @@ struct file_enum_count {
struct srvsvc_NetFileCtr3 *ctr3;
};
-struct sess_file_count {
- struct server_id pid;
- uid_t uid;
- int count;
-};
-
struct sess_file_info {
struct srvsvc_NetSessCtr1 *ctr;
struct sessionid *session_list;
@@ -812,38 +806,6 @@ static WERROR init_srv_sess_info_0(struct pipes_struct *p,
return WERR_OK;
}
-/*******************************************************************
-********************************************************************/
-
-static void sess_file_fn( const struct share_mode_entry *e,
- const char *sharepath, const char *fname,
- void *data )
-{
- struct sess_file_count *sess = (struct sess_file_count *)data;
-
- if (serverid_equal(&e->pid, &sess->pid) && (sess->uid == e->uid)) {
- sess->count++;
- }
-
- return;
-}
-
-/*******************************************************************
-********************************************************************/
-
-static int net_count_files( uid_t uid, struct server_id pid )
-{
- struct sess_file_count s_file_cnt;
-
- s_file_cnt.count = 0;
- s_file_cnt.uid = uid;
- s_file_cnt.pid = pid;
-
- share_mode_forall( sess_file_fn, &s_file_cnt );
-
- return s_file_cnt.count;
-}
-
/***********************************************************************
* find out the session on which this file is open and bump up its count
**********************************************************************/
@@ -936,25 +898,15 @@ static WERROR init_srv_sess_info_1(struct pipes_struct *p,
W_ERROR_HAVE_NO_MEMORY(ctr1->array);
for (num_entries = 0; resume_handle < *total_entries; num_entries++, resume_handle++) {
- uint32 num_files;
uint32 connect_time;
- struct passwd *pw = getpwnam(session_list[resume_handle].username);
bool guest;
- if ( !pw ) {
- DEBUG(10,("init_srv_sess_info_1: failed to find owner: %s\n",
- session_list[resume_handle].username));
- num_files = 0;
- } else {
- num_files = net_count_files(pw->pw_uid, session_list[resume_handle].pid);
- }
-
connect_time = (uint32_t)(now - session_list[resume_handle].connect_start);
guest = strequal( session_list[resume_handle].username, lp_guestaccount() );
ctr1->array[num_entries].client = session_list[resume_handle].remote_machine;
ctr1->array[num_entries].user = session_list[resume_handle].username;
- ctr1->array[num_entries].num_open = num_files;
+ ctr1->array[num_entries].num_open = 0;/* computed later */
ctr1->array[num_entries].time = connect_time;
ctr1->array[num_entries].idle_time = 0;
ctr1->array[num_entries].user_flags = guest;
@@ -962,6 +914,11 @@ static WERROR init_srv_sess_info_1(struct pipes_struct *p,
ctr1->count = num_entries;
+ /* count open files on all sessions in single tdb traversal */
+ net_count_files_for_all_sess(ctr1, session_list,
+ resume_handle_p ? *resume_handle_p : 0,
+ num_entries);
+
if (resume_handle_p) {
if (*resume_handle_p >= *total_entries) {
*resume_handle_p = 0;
--
1.8.5.2
More information about the samba-technical
mailing list