Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] CIFS: Fix SMB2 oplock break processing
@ 2019-10-31 21:18 Pavel Shilovsky
  2019-11-03  2:06 ` Steve French
  0 siblings, 1 reply; 2+ messages in thread
From: Pavel Shilovsky @ 2019-10-31 21:18 UTC (permalink / raw)
  To: linux-cifs; +Cc: stable

Even when mounting modern protocol version the server may be
configured without supporting SMB2.1 leases and the client
uses SMB2 oplock to optimize IO performance through local caching.

However there is a problem in oplock break handling that leads
to missing a break notification on the client who has a file
opened. It latter causes big latencies to other clients that
are trying to open the same file.

The problem reproduces when there are multiple shares from the
same server mounted on the client. The processing code tries to
match persistent and volatile file ids from the break notification
with an open file but it skips all share besides the first one.
Fix this by looking up in all shares belonging to the server that
issued the oplock break.

Cc: Stable <stable@vger.kernel.org>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/smb2misc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 8db6201b18ba..527c9efd3de0 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -664,10 +664,10 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
 	spin_lock(&cifs_tcp_ses_lock);
 	list_for_each(tmp, &server->smb_ses_list) {
 		ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
+
 		list_for_each(tmp1, &ses->tcon_list) {
 			tcon = list_entry(tmp1, struct cifs_tcon, tcon_list);
 
-			cifs_stats_inc(&tcon->stats.cifs_stats.num_oplock_brks);
 			spin_lock(&tcon->open_file_lock);
 			list_for_each(tmp2, &tcon->openFileList) {
 				cfile = list_entry(tmp2, struct cifsFileInfo,
@@ -679,6 +679,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
 					continue;
 
 				cifs_dbg(FYI, "file id match, oplock break\n");
+				cifs_stats_inc(
+				    &tcon->stats.cifs_stats.num_oplock_brks);
 				cinode = CIFS_I(d_inode(cfile->dentry));
 				spin_lock(&cfile->file_info_lock);
 				if (!CIFS_CACHE_WRITE(cinode) &&
@@ -702,9 +704,6 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
 				return true;
 			}
 			spin_unlock(&tcon->open_file_lock);
-			spin_unlock(&cifs_tcp_ses_lock);
-			cifs_dbg(FYI, "No matching file for oplock break\n");
-			return true;
 		}
 	}
 	spin_unlock(&cifs_tcp_ses_lock);
-- 
2.17.1


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

* Re: [PATCH] CIFS: Fix SMB2 oplock break processing
  2019-10-31 21:18 [PATCH] CIFS: Fix SMB2 oplock break processing Pavel Shilovsky
@ 2019-11-03  2:06 ` Steve French
  0 siblings, 0 replies; 2+ messages in thread
From: Steve French @ 2019-11-03  2:06 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: CIFS, Stable

tentatively merged into cifs-2.6.git for-next pending review and
buildbot regression test runs

On Thu, Oct 31, 2019 at 5:50 PM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> Even when mounting modern protocol version the server may be
> configured without supporting SMB2.1 leases and the client
> uses SMB2 oplock to optimize IO performance through local caching.
>
> However there is a problem in oplock break handling that leads
> to missing a break notification on the client who has a file
> opened. It latter causes big latencies to other clients that
> are trying to open the same file.
>
> The problem reproduces when there are multiple shares from the
> same server mounted on the client. The processing code tries to
> match persistent and volatile file ids from the break notification
> with an open file but it skips all share besides the first one.
> Fix this by looking up in all shares belonging to the server that
> issued the oplock break.
>
> Cc: Stable <stable@vger.kernel.org>
> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
> ---
>  fs/cifs/smb2misc.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 8db6201b18ba..527c9efd3de0 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -664,10 +664,10 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
>         spin_lock(&cifs_tcp_ses_lock);
>         list_for_each(tmp, &server->smb_ses_list) {
>                 ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
> +
>                 list_for_each(tmp1, &ses->tcon_list) {
>                         tcon = list_entry(tmp1, struct cifs_tcon, tcon_list);
>
> -                       cifs_stats_inc(&tcon->stats.cifs_stats.num_oplock_brks);
>                         spin_lock(&tcon->open_file_lock);
>                         list_for_each(tmp2, &tcon->openFileList) {
>                                 cfile = list_entry(tmp2, struct cifsFileInfo,
> @@ -679,6 +679,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
>                                         continue;
>
>                                 cifs_dbg(FYI, "file id match, oplock break\n");
> +                               cifs_stats_inc(
> +                                   &tcon->stats.cifs_stats.num_oplock_brks);
>                                 cinode = CIFS_I(d_inode(cfile->dentry));
>                                 spin_lock(&cfile->file_info_lock);
>                                 if (!CIFS_CACHE_WRITE(cinode) &&
> @@ -702,9 +704,6 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server)
>                                 return true;
>                         }
>                         spin_unlock(&tcon->open_file_lock);
> -                       spin_unlock(&cifs_tcp_ses_lock);
> -                       cifs_dbg(FYI, "No matching file for oplock break\n");
> -                       return true;
>                 }
>         }
>         spin_unlock(&cifs_tcp_ses_lock);
> --
> 2.17.1
>


-- 
Thanks,

Steve

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

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-31 21:18 [PATCH] CIFS: Fix SMB2 oplock break processing Pavel Shilovsky
2019-11-03  2:06 ` Steve French

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