| Message ID | 20210316124808.11984-1-vincent.whitchurch@axis.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] cifs: Silently ignore unknown oplock break handle | expand |
On 3/16/2021 8:48 AM, Vincent Whitchurch via samba-technical wrote: > Make SMB2 not print out an error when an oplock break is received for an > unknown handle, similar to SMB1. The SMB2 lease break path is not > affected by this patch. > > Without this, a program which writes to a file from one thread, and > opens, reads, and writes the same file from another thread triggers the > below errors several times a minute when run against a Samba server > configured with "smb2 leases = no". > > CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 > 00000000: 424d53fe 00000040 00000000 00000012 .SMB@........... > 00000010: 00000001 00000000 ffffffff ffffffff ................ > 00000020: 00000000 00000000 00000000 00000000 ................ > 00000030: 00000000 00000000 00000000 00000000 ................ > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > --- > > Notes: > v2: > - Drop change to lease break > - Rewrite commit message > > fs/cifs/smb2misc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > index 60d4bd1eae2b..4d8576e202e3 100644 > --- a/fs/cifs/smb2misc.c > +++ b/fs/cifs/smb2misc.c > @@ -755,7 +755,7 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > } > spin_unlock(&cifs_tcp_ses_lock); > cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); > - return false; > + return true; > } > > void > As an oplock-only approach, it looks good. But the old cifs_dbg message "non-existent connection" is possibly misleading, since the connection may be perfectly fine. When breaking the loop successfully, the code emits cifs_dbg(FYI, "file id match, oplock break\n"); so perhaps cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); ? Tom.
This issue will not be seen once changes related to deferred close for files is committed. Currently, changes are in review. I will address review comments by this week. Regards, Rohith On Tue, Mar 16, 2021 at 9:33 PM Tom Talpey <tom@talpey.com> wrote: > > On 3/16/2021 8:48 AM, Vincent Whitchurch via samba-technical wrote: > > Make SMB2 not print out an error when an oplock break is received for an > > unknown handle, similar to SMB1. The SMB2 lease break path is not > > affected by this patch. > > > > Without this, a program which writes to a file from one thread, and > > opens, reads, and writes the same file from another thread triggers the > > below errors several times a minute when run against a Samba server > > configured with "smb2 leases = no". > > > > CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 > > 00000000: 424d53fe 00000040 00000000 00000012 .SMB@........... > > 00000010: 00000001 00000000 ffffffff ffffffff ................ > > 00000020: 00000000 00000000 00000000 00000000 ................ > > 00000030: 00000000 00000000 00000000 00000000 ................ > > > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > > --- > > > > Notes: > > v2: > > - Drop change to lease break > > - Rewrite commit message > > > > fs/cifs/smb2misc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > > index 60d4bd1eae2b..4d8576e202e3 100644 > > --- a/fs/cifs/smb2misc.c > > +++ b/fs/cifs/smb2misc.c > > @@ -755,7 +755,7 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > > } > > spin_unlock(&cifs_tcp_ses_lock); > > cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); > > - return false; > > + return true; > > } > > > > void > > > > As an oplock-only approach, it looks good. But the old cifs_dbg message > "non-existent connection" is possibly misleading, since the connection > may be perfectly fine. > > When breaking the loop successfully, the code emits > cifs_dbg(FYI, "file id match, oplock break\n"); > so perhaps > cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); > ? > > Tom.
On 3/16/2021 1:36 PM, Rohith Surabattula wrote: > This issue will not be seen once changes related to deferred close for > files is committed. That may be, but it's irrelevant to this. > Currently, changes are in review. I will address review comments by this week. What do you mean by "in review"? Both threads are active on the mailing list. If you or others have something to discuss, please post it and don't leave us out of the discussion. Tom. > Regards, > Rohith > > On Tue, Mar 16, 2021 at 9:33 PM Tom Talpey <tom@talpey.com> wrote: >> >> On 3/16/2021 8:48 AM, Vincent Whitchurch via samba-technical wrote: >>> Make SMB2 not print out an error when an oplock break is received for an >>> unknown handle, similar to SMB1. The SMB2 lease break path is not >>> affected by this patch. >>> >>> Without this, a program which writes to a file from one thread, and >>> opens, reads, and writes the same file from another thread triggers the >>> below errors several times a minute when run against a Samba server >>> configured with "smb2 leases = no". >>> >>> CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 >>> 00000000: 424d53fe 00000040 00000000 00000012 .SMB@........... >>> 00000010: 00000001 00000000 ffffffff ffffffff ................ >>> 00000020: 00000000 00000000 00000000 00000000 ................ >>> 00000030: 00000000 00000000 00000000 00000000 ................ >>> >>> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> >>> --- >>> >>> Notes: >>> v2: >>> - Drop change to lease break >>> - Rewrite commit message >>> >>> fs/cifs/smb2misc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c >>> index 60d4bd1eae2b..4d8576e202e3 100644 >>> --- a/fs/cifs/smb2misc.c >>> +++ b/fs/cifs/smb2misc.c >>> @@ -755,7 +755,7 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) >>> } >>> spin_unlock(&cifs_tcp_ses_lock); >>> cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); >>> - return false; >>> + return true; >>> } >>> >>> void >>> >> >> As an oplock-only approach, it looks good. But the old cifs_dbg message >> "non-existent connection" is possibly misleading, since the connection >> may be perfectly fine. >> >> When breaking the loop successfully, the code emits >> cifs_dbg(FYI, "file id match, oplock break\n"); >> so perhaps >> cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); >> ? >> >> Tom. >
On 3/16/2021 1:36 PM, Rohith Surabattula wrote: >> This issue will not be seen once changes related to deferred close for >> files is committed. >That may be, but it's irrelevant to this. Got it. >> Currently, changes are in review. I will address review comments by this week. >What do you mean by "in review"? Both threads are active on the >mailing list. If you or others have something to discuss, please >post it and don't leave us out of the discussion. I mean, there is an active thread for patch "deferred close for files". As I was busy with other activities, I didn't update the thread. Will update the thread once i get to it. Regards, Rohith On Wed, Mar 17, 2021 at 12:59 AM Tom Talpey <tom@talpey.com> wrote: > > On 3/16/2021 1:36 PM, Rohith Surabattula wrote: > > This issue will not be seen once changes related to deferred close for > > files is committed. > > That may be, but it's irrelevant to this. > > > Currently, changes are in review. I will address review comments by this week. > > What do you mean by "in review"? Both threads are active on the > mailing list. If you or others have something to discuss, please > post it and don't leave us out of the discussion. > > Tom. > > > > Regards, > > Rohith > > > > On Tue, Mar 16, 2021 at 9:33 PM Tom Talpey <tom@talpey.com> wrote: > >> > >> On 3/16/2021 8:48 AM, Vincent Whitchurch via samba-technical wrote: > >>> Make SMB2 not print out an error when an oplock break is received for an > >>> unknown handle, similar to SMB1. The SMB2 lease break path is not > >>> affected by this patch. > >>> > >>> Without this, a program which writes to a file from one thread, and > >>> opens, reads, and writes the same file from another thread triggers the > >>> below errors several times a minute when run against a Samba server > >>> configured with "smb2 leases = no". > >>> > >>> CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 > >>> 00000000: 424d53fe 00000040 00000000 00000012 .SMB@........... > >>> 00000010: 00000001 00000000 ffffffff ffffffff ................ > >>> 00000020: 00000000 00000000 00000000 00000000 ................ > >>> 00000030: 00000000 00000000 00000000 00000000 ................ > >>> > >>> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > >>> --- > >>> > >>> Notes: > >>> v2: > >>> - Drop change to lease break > >>> - Rewrite commit message > >>> > >>> fs/cifs/smb2misc.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > >>> index 60d4bd1eae2b..4d8576e202e3 100644 > >>> --- a/fs/cifs/smb2misc.c > >>> +++ b/fs/cifs/smb2misc.c > >>> @@ -755,7 +755,7 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) > >>> } > >>> spin_unlock(&cifs_tcp_ses_lock); > >>> cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); > >>> - return false; > >>> + return true; > >>> } > >>> > >>> void > >>> > >> > >> As an oplock-only approach, it looks good. But the old cifs_dbg message > >> "non-existent connection" is possibly misleading, since the connection > >> may be perfectly fine. > >> > >> When breaking the loop successfully, the code emits > >> cifs_dbg(FYI, "file id match, oplock break\n"); > >> so perhaps > >> cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); > >> ? > >> > >> Tom. > >
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 60d4bd1eae2b..4d8576e202e3 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -755,7 +755,7 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } spin_unlock(&cifs_tcp_ses_lock); cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + return true; } void
Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 00000000: 424d53fe 00000040 00000000 00000012 .SMB@........... 00000010: 00000001 00000000 ffffffff ffffffff ................ 00000020: 00000000 00000000 00000000 00000000 ................ 00000030: 00000000 00000000 00000000 00000000 ................ Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> --- Notes: v2: - Drop change to lease break - Rewrite commit message fs/cifs/smb2misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)