Ran buildbot regression tests with the compounding patch and see
failure (failure is repeatable) with the full patch:
http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/812
but got it to pass with a smaller version of the same patch
http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/814
See attached diff, and updated (smaller version) of patch
Fixing compounding to use existing open handle is important, but would
be useful to understand why this fails
On Thu, Oct 14, 2021 at 12:35 PM Steve French <smfrench@xxxxxxxxx> wrote:
>
> tentatively merged the two you posted today into for-next pending testing etc.
>
> On Thu, Oct 14, 2021 at 5:41 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
> >
> > Hi Steve,
> >
> > I don't think this patch was taken. Can we take this?
> > https://github.com/sprasad-microsoft/smb3-kernel-client/commit/34c44cf16b97ee71b6d07720199b97ed328e7c97.patch
> >
> > Regards,
> > Shyam
> >
> > On Fri, Jul 30, 2021 at 10:44 PM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
> > >
> > > Hi Steve,
> > >
> > > Please review the patch.
> > > It fixes the issue of some compound requests unnecessarily breaking leases held by the same client.
> > >
> > > https://github.com/sprasad-microsoft/smb3-kernel-client/pull/2/commits/34bf1885b26db09a60bc276ea1a3f798f4cbb05f.patch
> > >
> > > We saw this yesterday when testing with generic/013 xfstests with the multichannel fixes.
> > >
> > > --
> > > Regards,
> > > Shyam
> >
> >
> >
> > --
> > Regards,
> > Shyam
>
>
>
> --
> Thanks,
>
> Steve
--
Thanks,
Steve
1c1
< From 98755a3543c0799ccbf2a35a92361adc47d844ab Mon Sep 17 00:00:00 2001
---
> From ae272e34bfa10495813171256101a49824d12dc7 Mon Sep 17 00:00:00 2001
16,17c16,17
< fs/cifs/smb2inode.c | 34 +++++++++++++++++++++++++++-------
< 1 file changed, 27 insertions(+), 7 deletions(-)
---
> fs/cifs/smb2inode.c | 42 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 33 insertions(+), 9 deletions(-)
20c20
< index 8297703492ee..69aeb54873f7 100644
---
> index 8297703492ee..beec42ff2bf9 100644
102,103c102,129
< --
< 2.32.0
---
> @@ -696,9 +711,12 @@ smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon,
> const char *from_name, const char *to_name,
> struct cifs_sb_info *cifs_sb)
> {
> + struct cifsFileInfo *cfile;
> +
> + cifs_get_writable_path(tcon, from_name, FIND_WR_ANY, &cfile);
> return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb,
> FILE_READ_ATTRIBUTES, SMB2_OP_HARDLINK,
> - NULL);
> + cfile);
> }
>
> int
> @@ -707,10 +725,12 @@ smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon,
> struct cifs_sb_info *cifs_sb, bool set_alloc)
> {
> __le64 eof = cpu_to_le64(size);
> + struct cifsFileInfo *cfile;
>
> + cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile);
> return smb2_compound_op(xid, tcon, cifs_sb, full_path,
> FILE_WRITE_DATA, FILE_OPEN, 0, ACL_NO_MODE,
> - &eof, SMB2_OP_SET_EOF, NULL);
> + &eof, SMB2_OP_SET_EOF, cfile);
> }
>
> int
From 98755a3543c0799ccbf2a35a92361adc47d844ab Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Date: Fri, 30 Jul 2021 06:43:09 +0000
Subject: [PATCH] cifs: for compound requests, use open handle if possible
For smb2_compound_op, it is possible to pass a ref to
an already open file. We should be passing it whenever possible.
i.e. if a matching handle is already kept open.
If we don't do that, we will end up breaking leases for files
kept open on the same client.
Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/cifs/smb2inode.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 8297703492ee..69aeb54873f7 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -46,6 +46,10 @@ struct cop_vars {
struct smb2_file_link_info link_info;
};
+/*
+ * note: If cfile is passed, the reference to it is dropped here.
+ * So make sure that you do not reuse cfile after return from this func.
+ */
static int
smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
struct cifs_sb_info *cifs_sb, const char *full_path,
@@ -536,10 +540,11 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon,
create_options |= OPEN_REPARSE_POINT;
/* Failed on a symbolic link - query a reparse point info */
+ cifs_get_readable_path(tcon, full_path, &cfile);
rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
FILE_READ_ATTRIBUTES, FILE_OPEN,
create_options, ACL_NO_MODE,
- smb2_data, SMB2_OP_QUERY_INFO, NULL);
+ smb2_data, SMB2_OP_QUERY_INFO, cfile);
}
if (rc)
goto out;
@@ -587,10 +592,11 @@ smb311_posix_query_path_info(const unsigned int xid, struct cifs_tcon *tcon,
create_options |= OPEN_REPARSE_POINT;
/* Failed on a symbolic link - query a reparse point info */
+ cifs_get_readable_path(tcon, full_path, &cfile);
rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
FILE_READ_ATTRIBUTES, FILE_OPEN,
create_options, ACL_NO_MODE,
- smb2_data, SMB2_OP_POSIX_QUERY_INFO, NULL);
+ smb2_data, SMB2_OP_POSIX_QUERY_INFO, cfile);
}
if (rc)
goto out;
@@ -608,10 +614,13 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode,
struct cifs_tcon *tcon, const char *name,
struct cifs_sb_info *cifs_sb)
{
+ struct cifsFileInfo *cfile;
+
+ cifs_get_writable_path(tcon, name, FIND_WR_ANY, &cfile);
return smb2_compound_op(xid, tcon, cifs_sb, name,
FILE_WRITE_ATTRIBUTES, FILE_CREATE,
CREATE_NOT_FILE, mode, NULL, SMB2_OP_MKDIR,
- NULL);
+ cfile);
}
void
@@ -642,18 +651,24 @@ int
smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
struct cifs_sb_info *cifs_sb)
{
+ struct cifsFileInfo *cfile;
+
+ cifs_get_writable_path(tcon, name, FIND_WR_WITH_DELETE, &cfile);
return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN,
CREATE_NOT_FILE, ACL_NO_MODE,
- NULL, SMB2_OP_RMDIR, NULL);
+ NULL, SMB2_OP_RMDIR, cfile);
}
int
smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
struct cifs_sb_info *cifs_sb)
{
+ struct cifsFileInfo *cfile;
+
+ cifs_get_writable_path(tcon, name, FIND_WR_WITH_DELETE, &cfile);
return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN,
CREATE_DELETE_ON_CLOSE | OPEN_REPARSE_POINT,
- ACL_NO_MODE, NULL, SMB2_OP_DELETE, NULL);
+ ACL_NO_MODE, NULL, SMB2_OP_DELETE, cfile);
}
static int
--
2.32.0
