Changes-for-displaying-proc-files-related-output by vedansh-bhardwaj · Pull Request #514 · namjaejeon/ksmbd

vedansh-bhardwaj · 2025-09-25T09:30:00Z

This Commit has Changes related to display Proc files output.

Please review and let us know if any changes required.
Thanks

mmakassikis

A few comments from reading the code. I haven't compiled it or checked it for correctness.

mmakassikis · 2025-09-28T16:05:32Z

+ccflags-y += -Wno-implicit-function-declaration
+ccflags-y += -Wno-incompatible-pointer-types
+ccflags-y += -Wno-unused-variable
+


these are good warnings to enable, but unrelated to the feature you are implementing. Perhaps they should be in a separate commit ?

Removed these changes by fixing warning issues.

mmakassikis · 2025-09-28T16:10:27Z

+{
+	if (user->name[0] == '\0')
+		return 1;
+	return 0;


this can be shortened to:

return user->name[0] == '\0';

and the return type should be bool, no ?

mmakassikis · 2025-09-28T16:30:51Z

+	return session->user->name;
+}
+
+static int show_proc_session(struct seq_file *m, void *v)


Isn't this function racy ?
the commented get_session() / put_session() hint that some locking to protect struct ksmbd_session from being freed.
Perhaps you meant ksmbd_user_session_get() and ksmbd_user_session_put() ?

Fixed this.

mmakassikis · 2025-09-28T16:35:54Z

+	seq_printf(m, "#%-40s %-15s %-10s %-10s\n",
+		 "<client>", "<user>", "<sess_id>", "<state>");
+
+	down_read(&sessions_table_lock);


I don't know if this is enough. Code in mgmt/user_session.c that locks sessions_table_lock also locks the conn->session_lock.

mmakassikis · 2025-09-28T16:36:33Z

+
+	down_read(&sessions_table_lock);
+	hash_for_each(sessions_table, i, session, hlist) {
+		// get_session(session);


Is that a leftover, or do you need to use ksmbd_user_session_get() and ksmbd_user_session_put() ?

mmakassikis · 2025-09-28T16:45:11Z

 		 * on reauthetication.
 		 */
-		if (conn->binding == false && ksmbd_anonymous_user(user)) {
+		if (conn->binding == false && user_anonymous(user)) {


I hadn't noticed user_anonymous() is ksmbd_anonymous_user(). I guess it's more coherent with user_guest(), but shouldn't that be a separate commit ?

Reverted this changes

mmakassikis · 2025-09-28T16:47:42Z

+#ifndef __KSMBD_STATS_H__
+#define __KSMBD_STATS_H__
+
+#define KSMBD_COUNTER_MAX_REQS	19


Can you add an explanation as to where the magic value comes from ? (even better: is it possible to get rid of it ?)

There are 19 SMB2 commands defined in smb2pdu.h file

mmakassikis · 2025-09-28T16:49:21Z

+#else
+struct ksmbd_counters {
+	int counters[0];
+};


is it necessary to declare ksmbd_counters in the !PROCFS case ?

mmakassikis · 2025-09-28T16:49:54Z

+
 	handler = kthread_run(ksmbd_conn_handler_loop,
-			      KSMBD_TRANS(t)->conn,
+			      conn,


this looks like code from a different commit

mmakassikis · 2025-09-28T16:51:00Z

+#define SMB2_LEASE_NONE_LE			cpu_to_le32(0x00)
+#define SMB2_LEASE_READ_CACHING_LE		cpu_to_le32(0x01)
+#define SMB2_LEASE_HANDLE_CACHING_LE		cpu_to_le32(0x02)
+#define SMB2_LEASE_WRITE_CACHING_LE		cpu_to_le32(0x04)


use the values from fs/smb/common/smb2pdu.h

vedansh-bhardwaj · 2025-10-03T05:13:59Z

Hi @mmakassikis Thanks for your inputs currently i am doing changes according to your inputs i will update here once changes are done.

Thanks

vedansh-bhardwaj · 2025-10-09T08:51:07Z

Hi @mmakassikis made the changes as per your input. Please review and let us know if any changes required.

Thanks

namjaejeon · 2025-10-10T03:31:10Z

@vedansh-bhardwaj Can you run checkpatch.pl and fix the warnings ? there is checkpatch.pl script in linux kernel source. and please use --strict option when running it.

$> smb3-kernel/scripts/checkpatch.pl --strict 0001-Changes-for-displaying-proc-files-related-output.patchTraceback (most recent call last):

ERROR: do not set execute permissions for source files
#42: FILE: Makefile

ERROR: do not set execute permissions for source files
#56: FILE: connection.c

CHECK: Blank lines aren't necessary before a close brace '}'
#97: FILE: connection.c:54:
+
+	}

CHECK: Alignment should match open parenthesis
#105: FILE: connection.c:62:
+	proc_clients = ksmbd_proc_create("clients",
+				proc_show_clients, NULL);

CHECK: Comparison to NULL could be written "!proc_clients"
#106: FILE: connection.c:63:
+	if (proc_clients == NULL)

ERROR: do not set execute permissions for source files
#242: FILE: mgmt/user_session.c

WARNING: suspect code indent for conditional statements (16, 12)
#376: FILE: mgmt/user_session.c:146:
+		if (table[i].const_value & flags) {
+            seq_printf(m, "0x%08x\t", table[i].const_value);

WARNING: braces {} are not necessary for single statement blocks
#376: FILE: mgmt/user_session.c:146:
+		if (table[i].const_value & flags) {
+            seq_printf(m, "0x%08x\t", table[i].const_value);
+        }

ERROR: code indent should use tabs where possible
#377: FILE: mgmt/user_session.c:147:
+            seq_printf(m, "0x%08x\t", table[i].const_value);$

WARNING: please, no spaces at the start of a line
#377: FILE: mgmt/user_session.c:147:
+            seq_printf(m, "0x%08x\t", table[i].const_value);$

ERROR: code indent should use tabs where possible
#378: FILE: mgmt/user_session.c:148:
+        }$

WARNING: please, no spaces at the start of a line
#378: FILE: mgmt/user_session.c:148:
+        }$

WARNING: suspect code indent for conditional statements (16, 12)
#392: FILE: mgmt/user_session.c:162:
+		if (table[i].const_value & const_value) {
+            seq_printf(m, format, table[i].name);

WARNING: braces {} are not necessary for single statement blocks
#392: FILE: mgmt/user_session.c:162:
+		if (table[i].const_value & const_value) {
+            seq_printf(m, format, table[i].name);
+        }

ERROR: code indent should use tabs where possible
#393: FILE: mgmt/user_session.c:163:
+            seq_printf(m, format, table[i].name);$

WARNING: please, no spaces at the start of a line
#393: FILE: mgmt/user_session.c:163:
+            seq_printf(m, format, table[i].name);$

ERROR: code indent should use tabs where possible
#394: FILE: mgmt/user_session.c:164:
+        }$

WARNING: please, no spaces at the start of a line
#394: FILE: mgmt/user_session.c:164:
+        }$

CHECK: Alignment should match open parenthesis
#420: FILE: mgmt/user_session.c:190:
+	seq_printf(m, "#%-40s %-15s %-10s %-10s\n",
+		 "<client>", "<user>", "<sess_id>", "<state>");

CHECK: Please don't use multiple blank lines
#535: FILE: misc.h:13:
+
+

CHECK: Alignment should match open parenthesis
#554: FILE: misc.h:53:
+struct proc_dir_entry *ksmbd_proc_create(const char *name,
+			     int (*show)(struct seq_file *m, void *v),

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#572: 
new file mode 100644

CHECK: Alignment should match open parenthesis
#596: FILE: proc.c:20:
+struct proc_dir_entry *ksmbd_proc_create(const char *name,
+			     int (*show)(struct seq_file *m, void *v),

ERROR: space prohibited before open square bracket '['
#608: FILE: proc.c:32:
+static const struct ksmbd_const_smb2_process_req smb2_process_req [KSMBD_COUNTER_MAX_REQS] = {

ERROR: space prohibited before that ',' (ctx:WxE)
#654: FILE: proc.c:78:
+		seq_printf(m, "%-20s:\t%lld\n", smb2_process_req[i].name ,
 		                                                         ^

CHECK: Comparison to NULL could be written "!ksmbd_proc_fs"
#663: FILE: proc.c:87:
+	if (ksmbd_proc_fs == NULL)

CHECK: Comparison to NULL could be written "!ksmbd_proc_fs"
#688: FILE: proc.c:112:
+	if (ksmbd_proc_fs == NULL)

CHECK: Comparison to NULL could be written "!proc_mkdir_mode"
#691: FILE: proc.c:115:
+	if (proc_mkdir_mode("sessions", 0400, ksmbd_proc_fs) == NULL)

CHECK: Lines should not end with a '('
#695: FILE: proc.c:119:
+		retval = percpu_counter_init(

CHECK: Comparison to NULL could be written "!ksmbd_proc_create"
#701: FILE: proc.c:125:
+	if (ksmbd_proc_create("server", proc_show_ksmbd_stats, NULL) == NULL)

CHECK: Please don't use multiple blank lines
#726: FILE: server.c:33:
+
+

CHECK: Please don't use multiple blank lines
#822: FILE: smb2pdu.c:240:
+
+

CHECK: Please don't use multiple blank lines
#871: FILE: smb_common.c:151:
+
+

CHECK: Lines should not end with a '('
#954: FILE: stats.h:54:
+		percpu_counter_inc(

WARNING: line length of 102 exceeds 100 columns
#983: FILE: transport_tcp.c:232:
+		snprintf(KSMBD_TRANS(t)->conn->client_name, sizeof(KSMBD_TRANS(t)->conn->client_name),

WARNING: line length of 102 exceeds 100 columns
#990: FILE: transport_tcp.c:238:
+		snprintf(KSMBD_TRANS(t)->conn->client_name, sizeof(KSMBD_TRANS(t)->conn->client_name),

CHECK: Blank lines aren't necessary after an open brace '{'
#1098: FILE: vfs_cache.c:92:
+	idr_for_each_entry(global_ft.idr, fp, id) {
+

CHECK: braces {} should be used on all arms of this statement
#1105: FILE: vfs_cache.c:99:
+		if (!fp->f_opinfo)
[...]
+		else {
[...]

CHECK: Unbalanced braces around else statement
#1107: FILE: vfs_cache.c:101:
+		else {

total: 9 errors, 12 warnings, 19 checks, 977 lines checked

vedansh-bhardwaj · 2025-10-10T05:19:32Z

Hi @namjaejeon I will check and update you.

Thanks

vedansh-bhardwaj · 2025-10-15T14:46:10Z

Hi @namjaejeon we fixed the warnings and errors except this below mentioned warning.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#568:
new file mode 100644

Please review.

Thanks

namjaejeon · 2025-10-16T01:33:28Z

@vedansh-bhardwaj Okay, I will check it. Thanks!

namjaejeon · 2025-10-17T06:59:25Z

 		ksmbd_release_id(&sess->tree_conn_ida, id);
 }
+
+int ksmbd_sessions_init(void)


we don't need this wrapper function. please open code and directly use create_proc_sessions().

namjaejeon · 2025-10-17T07:08:36Z

+#ifdef CONFIG_PROC_FS
+	struct proc_dir_entry		*proc_entry;
+#endif
+	struct ksmbd_conn		*conn;


Please don't add conn here.
session can have several connection. So it is called as channel. So you can access conn through channel.

namjaejeon · 2025-10-17T07:13:20Z

 	__le16				signing_algorithm;
 	bool				binding;
+
+	char				client_name[INET6_ADDRSTRLEN + 1];


Should it consume 49bytes per connection ? can we use inet6_addr or inet_addr ?

namjaejeon · 2025-10-17T07:14:20Z

+	else if (session->state == SMB2_SESSION_EXPIRED)
+		return "expired";
+	else
+		return "";


How about use switch condition instead of if ?

namjaejeon · 2025-10-17T07:15:25Z

+	}
+
+	i = 0;
+	xa_for_each(&sess->ksmbd_chann_list, id, chan) {


we can remove {} if loop is single line.

namjaejeon · 2025-10-17T07:18:34Z

+
+struct proc_dir_entry *ksmbd_proc_create(const char *name,
+					 int (*show)(struct seq_file *m, void *v),
+			     void *v)


I prefer two tab and align it

When i tried to add two tab and align it.

I am getting this Check.

vedansh@ws2030077069:~/ksmbd_vedansh/ksmbd$ ~/smb3-kernel/scripts/checkpatch.pl --strict 0001-Required-changes-for-displaying-proc-related-output.patch
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#551:
new file mode 100644

CHECK: Alignment should match open parenthesis
#582: FILE: proc.c:27:
+struct proc_dir_entry *ksmbd_proc_create(const char *name,

int (*show)(struct seq_file *m, void *v), void *v)

total: 0 errors, 1 warnings, 1 checks, 919 lines checked

namjaejeon · 2025-10-17T07:19:36Z

+	seq_printf(m, "written bytes: %lld\n",
+		   ksmbd_counter_sum(KSMBD_COUNTER_WRITE_BYTES));
+
+	seq_puts(m, "\nSmb2\n");


Smb2 -> SMB2

namjaejeon · 2025-10-17T07:21:36Z

 	ret = ksmbd_workqueue_init();
 	if (ret)
 		goto err_crypto_destroy;
+


Please remove this line.

namjaejeon · 2025-10-17T07:23:24Z

 	return le16_to_cpu(rcv_hdr->Command);
 }

+void smb2_inc_reqs(unsigned int cmd)


ditto, we can remove wrapper function and open code.

namjaejeon · 2025-10-17T07:26:59Z

+
+static const struct ksmbd_const_name ksmbd_lease_const_names[] = {
+	{le32_to_cpu(SMB2_LEASE_NONE_LE),
+		"LEASE_NONE"},


Is there any reason why you put "LEASE_NONE" in next line ? the line is over 80 ?

vedansh-bhardwaj · 2025-10-20T06:32:37Z

Hi @namjaejeon
I will do the needful changes and update you.
Thanks

namjaejeon · 2025-12-09T08:02:11Z

@vedansh-bhardwaj Okay, Looks good to me, BTW, there are so many signed-off-by and authors.

Signed-off-by: v.bhardwaj v.bhardwaj@samsung.com
Signed-off-by: bahubali.bg bahubali.bg@samsung.com
Signed-off-by: m.sataraddi m.sataraddi@samsung.com
Signed-off-by: c.singh c.singh@samsung.com
Signed-off-by: constant.lee constant.lee@samsung.com
Signed-off-by: moons49.kim moons49.kim@samsung.com

- Author(s):v.bhardwaj v.bhardwaj@samsung.com
- Author(s):bahubali.bg bahubali.bg@samsung.com
- Author(s):m.sataraddi m.sataraddi@samsung.com
- Author(s):c.singh c.singh@samsung.com
- Author(s):constant.lee constant.lee@samsung.com
- Author(s):moons49.kim moons49.kim@samsung.com

I don't think all of these people were involved. Please make signed-off-by up to 2 people and author up to 1 person.
Thanks.

vedansh-bhardwaj · 2025-12-09T09:39:59Z

Hi @namjaejeon we will update.
Thanks

vedansh-bhardwaj · 2025-12-15T11:21:32Z

Hi @namjaejeon
updated the signed-off and authors details. Please review.
Thanks

This patch updates the logic used to display information through the proc files in ksmbd. The goal is to make the output more readable, structured, and aligned with kernel formatting conventions. Key changes include: - Refactored code handling proc file reads for better maintainability. - Updated data structures to ensure proper display of stats and status. - Improved consistency of field names and value formatting. Signed-off-by: Bahubali B Gumaji <bahubali.bg@samsung.com> Signed-off-by: Sang-Soo Lee <constant.lee@samsung.com>

namjaejeon · 2025-12-18T05:23:00Z

@vedansh-bhardwaj I have updated your patch like this (18abb79).
I changed %pI6 and %pl4 to %pl6c and %pl4c. It will not show hex ip address. Is it okay for your target ?

vedansh-bhardwaj · 2025-12-18T07:44:48Z

Hi @namjaejeon
It is ok for us.
Thanks

vedansh-bhardwaj · 2026-01-13T10:27:22Z

Hi @namjaejeon
Happy new year

Any update on these proc related changes.

thanks

namjaejeon · 2026-01-14T00:03:59Z

@vedansh-bhardwaj The proc patches are in next branch. It will be applied to master when linux 6.20-rc1 merge window open.

vedansh-bhardwaj · 2026-01-15T07:02:31Z

Hi @namjaejeon
Thanks for update.

vedansh-bhardwaj mentioned this pull request Sep 25, 2025

Request | Provision in ksmbd-tools to know details about connected KSMBD Clients cifsd-team/ksmbd-tools#329

Open

mmakassikis reviewed Sep 28, 2025

View reviewed changes

vedansh-bhardwaj closed this Oct 7, 2025

vedansh-bhardwaj force-pushed the master branch from 0596001 to aa9ec35 Compare October 7, 2025 10:25

vedansh-bhardwaj reopened this Oct 9, 2025

vedansh-bhardwaj force-pushed the master branch from 576fd2c to c39ec95 Compare October 15, 2025 14:26

namjaejeon reviewed Oct 17, 2025

View reviewed changes

vedansh-bhardwaj force-pushed the master branch from c39ec95 to 02c82e4 Compare November 17, 2025 12:23

vedansh-bhardwaj force-pushed the master branch from 02c82e4 to 9fd563f Compare December 15, 2025 11:16

vedansh-bhardwaj force-pushed the master branch from 9fd563f to ec390cc Compare December 16, 2025 05:15

vedansh-bhardwaj force-pushed the master branch from ec390cc to 2ab60a9 Compare December 16, 2025 10:10

Conversation

vedansh-bhardwaj commented Sep 25, 2025

Uh oh!

mmakassikis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vedansh-bhardwaj commented Oct 3, 2025

Uh oh!

vedansh-bhardwaj commented Oct 9, 2025

Uh oh!

namjaejeon commented Oct 10, 2025

Uh oh!

vedansh-bhardwaj commented Oct 10, 2025

Uh oh!

vedansh-bhardwaj commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

namjaejeon commented Oct 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

vedansh-bhardwaj commented Oct 15, 2025 •

edited

Loading