Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH][SMB3] Increment num_remote_opens stats counter even in case of smb2_query_dir_first
@ 2020-03-11  4:36 Steve French
  2020-03-11 11:14 ` Aurélien Aptel
  0 siblings, 1 reply; 3+ messages in thread
From: Steve French @ 2020-03-11  4:36 UTC (permalink / raw)
  To: Shyam Prasad N, CIFS

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

Patch from Shyam attached (tentatively merged into cifs-2.6.git
for-next). See his description below:

The num_remote_opens counter keeps track of the number of open files
which must be maintained by the server at any point. This is a
per-tree-connect counter, and the value of this counter gets displayed
in the /proc/fs/cifs/Stats output as a following...

    Open files: 0 total (local), 1 open on server
                                 ^^^^^^^^^^^^^^^^
    As a thumb-rule, we want to increment this counter for each
open/create that we successfully execute on the server. Similarly, we
should decrement the counter when we successfully execute a close.

    In this case, an increment was being missed in case of smb2_query_dir_first,
    in case of successful open. As a result, we would underflow the
counter and we could even see the counter go to negative after
sufficient smb2_query_dir_first calls.

    I tested the stats counter for a bunch of filesystem operations
with the fix.
    And it looks like the counter looks correct to me.

    I also check if we missed the increments and decrements elsewhere.
It does not seem so. Few other cases where an open is done and we
don't increment the counter are
    the compound calls where the corresponding close is also sent in
the request.

-- 
Thanks,

Steve

[-- Attachment #2: 0001-CIFS-Increment-num_remote_opens-stats-counter-even-i.patch --]
[-- Type: text/x-patch, Size: 2115 bytes --]

From f34d2f84852219dd8b252f08773f4e49c3b2db4d Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <nspmangalore@gmail.com>
Date: Mon, 9 Mar 2020 01:35:09 -0700
Subject: [PATCH] CIFS: Increment num_remote_opens stats counter even in case
 of smb2_query_dir_first

The num_remote_opens counter keeps track of the number of open files which must be
maintained by the server at any point. This is a per-tree-connect counter, and the value
of this counter gets displayed in the /proc/fs/cifs/Stats output as a following...

Open files: 0 total (local), 1 open on server
                             ^^^^^^^^^^^^^^^^
As a thumb-rule, we want to increment this counter for each open/create that we
successfully execute on the server. Similarly, we should decrement the counter when
we successfully execute a close.

In this case, an increment was being missed in case of smb2_query_dir_first,
in case of successful open. As a result, we would underflow the counter and we
could even see the counter go to negative after sufficient smb2_query_dir_first calls.

I tested the stats counter for a bunch of filesystem operations with the fix.
And it looks like the counter looks correct to me.

I also check if we missed the increments and decrements elsewhere. It does not
seem so. Few other cases where an open is done and we don't increment the counter are
the compound calls where the corresponding close is also sent in the request.

Signed-off-by: Shyam Prasad N <nspmangalore@gmail.com>
CC: Stable <stable@vger.kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2ops.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 83e5b351d5af..ea9fcdca4d20 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2226,6 +2226,8 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
 		goto qdf_free;
 	}
 
+	atomic_inc(&tcon->num_remote_opens);
+
 	qd_rsp = (struct smb2_query_directory_rsp *)rsp_iov[1].iov_base;
 	if (qd_rsp->sync_hdr.Status == STATUS_NO_MORE_FILES) {
 		trace_smb3_query_dir_done(xid, fid->persistent_fid,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][SMB3] Increment num_remote_opens stats counter even in case of smb2_query_dir_first
  2020-03-11  4:36 [PATCH][SMB3] Increment num_remote_opens stats counter even in case of smb2_query_dir_first Steve French
@ 2020-03-11 11:14 ` Aurélien Aptel
  2020-03-11 17:34   ` Pavel Shilovsky
  0 siblings, 1 reply; 3+ messages in thread
From: Aurélien Aptel @ 2020-03-11 11:14 UTC (permalink / raw)
  To: Steve French, Shyam Prasad N, CIFS

Good catch! Usually compounded chains do both open and close so they
don't need to update the counter but this one doesn't close.

Reviewed-by: Aurelien Aptel <aaptel@suse.com>

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][SMB3] Increment num_remote_opens stats counter even in case of smb2_query_dir_first
  2020-03-11 11:14 ` Aurélien Aptel
@ 2020-03-11 17:34   ` Pavel Shilovsky
  0 siblings, 0 replies; 3+ messages in thread
From: Pavel Shilovsky @ 2020-03-11 17:34 UTC (permalink / raw)
  To: Aurélien Aptel; +Cc: Steve French, Shyam Prasad N, CIFS

Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>

--
Best regards,
Pavel Shilovsky

ср, 11 мар. 2020 г. в 04:14, Aurélien Aptel <aaptel@suse.com>:
>
> Good catch! Usually compounded chains do both open and close so they
> don't need to update the counter but this one doesn't close.
>
> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
>
> Cheers,
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11  4:36 [PATCH][SMB3] Increment num_remote_opens stats counter even in case of smb2_query_dir_first Steve French
2020-03-11 11:14 ` Aurélien Aptel
2020-03-11 17:34   ` Pavel Shilovsky

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org
	public-inbox-index linux-cifs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git