On Fri, Oct 28, 2022 at 2:49 PM ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote: > > On Fri, 28 Oct 2022 at 16:55, ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote: > > > > On Fri, 28 Oct 2022 at 16:53, Steve French <smfrench@xxxxxxxxx> wrote: > > > > > > I haven't tried a scenario to windows where we turn off leases and force server to use oplocks but with ksmbd that is the default. > > > Worth also investigating how primary vs secondary works for finding leases for windows case > > > > Yes. Until we know what/how windows does things and what ms-smb2.pdf > > says we can not know if this is a cifs client bug or a ksmbd bug. > > So lets wait with this patch until we know where the bug is. > I will review it later for locking correctness, but lets make sure it > is not a ksmbd bug first. > > > > > > > > > > > > On Fri, Oct 28, 2022, 01:48 ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote: > > >> > > >> On Fri, 28 Oct 2022 at 16:25, Steve French <smfrench@xxxxxxxxx> wrote: > > >> > > > >> > If a mount to a server is using multichannel, an oplock break arriving > > >> > on a secondary channel won't find the open file (since it won't find the > > >> > tcon for it), and this will cause each oplock break on secondary channels > > >> > to time out, slowing performance drastically. > > >> > > > >> > Fix smb2_is_valid_oplock_break so that if it is a secondary channel and > > >> > an oplock break was not found, check for tcons (and the files hanging > > >> > off the tcons) on the primary channel. > > >> > > >> Does this also happen against windows or is is only against ksmbd this triggers? > > >> What does MS-SMB2.pdf say about channels and oplocks? > > >> > > >> > > > >> > Fixes xfstest generic/013 to ksmbd > > >> > > > >> > Cc: <stable@xxxxxxxxxxxxxxx> > > >> > > > >> > > > >> > -- > > >> > Thanks, > > >> > > > >> > Steve This is a valid bug, and needs to be fixed. Here's my version of the fix. Few more places needed corrections. Please review: https://github.com/sprasad-microsoft/smb3-kernel-client/commit/2b57cec4d03464f4875671d4146e75fb41476941.patch I think I know why this is an issue only for oplocks and not leases. Both is_valid_oplock_break and smb2_is_valid_oplock_break pass server (on which the notification was received) as an arg. However, for some strange reason, smb2_is_valid_lease_break does not. That is what saved lease break from this bug. I think passing the server as an arg even for lease break check will save some unnecessary iterations. I'm seeing similar behaviour in couple of other places. Fixed them here: https://github.com/sprasad-microsoft/smb3-kernel-client/commit/6bfba4f4939afb36364e6c4f4f8ca979cd526729.patch Thoughts? -- Regards, Shyam
