| Message ID | 20231204045632.72226-1-meetakshisetiyaoss@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | cifs: Reuse file lease key in compound operations | expand |
On Mon, Dec 4, 2023 at 10:26 AM <meetakshisetiyaoss@gmail.com> wrote: > > From: Meetakshi Setiya <msetiya@microsoft.com> > > Lock contention during unlink operation causes cifs lease break ack > worker thread to block and delay sending lease break acks to server. > This case occurs when multiple threads perform unlink, write and lease > break acks on the same file. Thhis patch fixes the problem by reusing > the existing lease keys for rename, unlink and set path size compound > operations so that the client does not break its own lease. I think we can be more clear on when the issue can occur: The issue can be seen when a file has a lot of dirty pages to be written to the server. When a rename, unlink or set path size compound operation is requested on this file, we do not send the lease key for these requests. As a result, the server can assume that this request is from a new client, and send a lease break notification to the same client, on the same connection. As a response to the lease break, the client can consume several credits to write the dirty pages to the server. Depending on the server's credit grant implementation, the server can stop granting more credits to this connection, and this can cause a deadlock (which can only be resolved when the lease timer on the server expires). One of the problems here is that the client is sending no lease key, even if it has a lease for the file. Thhis patch fixes the problem by reusing the existing lease keys for rename, unlink and set path size compound operations so that the client does not break its own lease. > > Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> > --- > fs/smb/client/cifsglob.h | 6 ++--- > fs/smb/client/cifsproto.h | 8 +++---- > fs/smb/client/cifssmb.c | 6 ++--- > fs/smb/client/inode.c | 12 +++++----- > fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- > fs/smb/client/smb2proto.h | 8 +++---- > 6 files changed, 51 insertions(+), 38 deletions(-) > > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h > index 7558167f603c..3f6f993357bd 100644 > --- a/fs/smb/client/cifsglob.h > +++ b/fs/smb/client/cifsglob.h > @@ -355,7 +355,7 @@ struct smb_version_operations { > struct cifs_open_info_data *data); > /* set size by path */ > int (*set_path_size)(const unsigned int, struct cifs_tcon *, > - const char *, __u64, struct cifs_sb_info *, bool); > + const char *, __u64, struct cifs_sb_info *, bool, struct dentry *); > /* set size by file handle */ > int (*set_file_size)(const unsigned int, struct cifs_tcon *, > struct cifsFileInfo *, __u64, bool); > @@ -385,13 +385,13 @@ struct smb_version_operations { > struct cifs_sb_info *); > /* unlink file */ > int (*unlink)(const unsigned int, struct cifs_tcon *, const char *, > - struct cifs_sb_info *); > + struct cifs_sb_info *, struct dentry *); > /* open, rename and delete file */ > int (*rename_pending_delete)(const char *, struct dentry *, > const unsigned int); > /* send rename request */ > int (*rename)(const unsigned int, struct cifs_tcon *, const char *, > - const char *, struct cifs_sb_info *); > + const char *, struct cifs_sb_info *, struct dentry *); > /* send create hardlink request */ > int (*create_hardlink)(const unsigned int, struct cifs_tcon *, > const char *, const char *, > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h > index 46feaa0880bd..3bb15cc74bc2 100644 > --- a/fs/smb/client/cifsproto.h > +++ b/fs/smb/client/cifsproto.h > @@ -397,8 +397,8 @@ extern int CIFSSMBSetFileDisposition(const unsigned int xid, > bool delete_file, __u16 fid, > __u32 pid_of_opener); > extern int CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, > - const char *file_name, __u64 size, > - struct cifs_sb_info *cifs_sb, bool set_allocation); > + const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, > + bool set_allocation, struct dentry *dentry); > extern int CIFSSMBSetFileSize(const unsigned int xid, struct cifs_tcon *tcon, > struct cifsFileInfo *cfile, __u64 size, > bool set_allocation); > @@ -434,10 +434,10 @@ extern int CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, > const struct nls_table *nls_codepage, > int remap_special_chars); > extern int CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, > - const char *name, struct cifs_sb_info *cifs_sb); > + const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry); > extern int CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > - struct cifs_sb_info *cifs_sb); > + struct cifs_sb_info *cifs_sb, struct dentry *dentry); > extern int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *tcon, > int netfid, const char *target_name, > const struct nls_table *nls_codepage, > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c > index 9ee348e6d106..023b3bfa7b94 100644 > --- a/fs/smb/client/cifssmb.c > +++ b/fs/smb/client/cifssmb.c > @@ -738,7 +738,7 @@ CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, > > int > CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > - struct cifs_sb_info *cifs_sb) > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > { > DELETE_FILE_REQ *pSMB = NULL; > DELETE_FILE_RSP *pSMBr = NULL; > @@ -2152,7 +2152,7 @@ CIFSSMBFlush(const unsigned int xid, struct cifs_tcon *tcon, int smb_file_id) > int > CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > - struct cifs_sb_info *cifs_sb) > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > { > int rc = 0; > RENAME_REQ *pSMB = NULL; > @@ -4982,7 +4982,7 @@ CIFSSMBQFSPosixInfo(const unsigned int xid, struct cifs_tcon *tcon, > int > CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, > const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, > - bool set_allocation) > + bool set_allocation, struct dentry *dentry) > { > struct smb_com_transaction2_spi_req *pSMB = NULL; > struct smb_com_transaction2_spi_rsp *pSMBr = NULL; > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c > index d01e9ea67ccd..d5ad54733637 100644 > --- a/fs/smb/client/inode.c > +++ b/fs/smb/client/inode.c > @@ -1828,7 +1828,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) > goto psx_del_no_retry; > } > > - rc = server->ops->unlink(xid, tcon, full_path, cifs_sb); > + rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry); > > psx_del_no_retry: > if (!rc) { > @@ -2227,7 +2227,7 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry, > return -ENOSYS; > > /* try path-based rename first */ > - rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb); > + rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb, from_dentry); > > /* > * Don't bother with rename by filehandle unless file is busy and > @@ -2774,7 +2774,7 @@ void cifs_setsize(struct inode *inode, loff_t offset) > > static int > cifs_set_file_size(struct inode *inode, struct iattr *attrs, > - unsigned int xid, const char *full_path) > + unsigned int xid, const char *full_path, struct dentry *dentry) > { > int rc; > struct cifsFileInfo *open_file; > @@ -2825,7 +2825,7 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs, > */ > if (server->ops->set_path_size) > rc = server->ops->set_path_size(xid, tcon, full_path, > - attrs->ia_size, cifs_sb, false); > + attrs->ia_size, cifs_sb, false, dentry); > else > rc = -ENOSYS; > cifs_dbg(FYI, "SetEOF by path (setattrs) rc = %d\n", rc); > @@ -2915,7 +2915,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) > rc = 0; > > if (attrs->ia_valid & ATTR_SIZE) { > - rc = cifs_set_file_size(inode, attrs, xid, full_path); > + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); > if (rc != 0) > goto out; > } > @@ -3081,7 +3081,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) > } > > if (attrs->ia_valid & ATTR_SIZE) { > - rc = cifs_set_file_size(inode, attrs, xid, full_path); > + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); > if (rc != 0) > goto cifs_setattr_exit; > } > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c > index c94940af5d4b..ebee4779c743 100644 > --- a/fs/smb/client/smb2inode.c > +++ b/fs/smb/client/smb2inode.c > @@ -48,7 +48,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > __u32 desired_access, __u32 create_disposition, __u32 create_options, > umode_t mode, void *ptr, int command, struct cifsFileInfo *cfile, > __u8 **extbuf, size_t *extbuflen, > - struct kvec *out_iov, int *out_buftype) > + struct kvec *out_iov, int *out_buftype, struct dentry *dentry) > { > struct smb2_compound_vars *vars = NULL; > struct kvec *rsp_iov; > @@ -59,6 +59,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > struct cifs_fid fid; > struct cifs_ses *ses = tcon->ses; > struct TCP_Server_Info *server; > + struct inode *inode = NULL; > + struct cifsInodeInfo *cinode = NULL; > int num_rqst = 0; > int resp_buftype[3]; > struct smb2_query_info_rsp *qi_rsp = NULL; > @@ -93,6 +95,16 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > goto finished; > } > > + //if there is an existing lease, reuse it > + if (dentry) { > + inode = d_inode(dentry); > + cinode = CIFS_I(inode); > + if (cinode->lease_granted) { > + oplock = SMB2_OPLOCK_LEVEL_LEASE; > + memcpy(fid.lease_key, cinode->lease_key, SMB2_LEASE_KEY_SIZE); > + } > + } > + > vars->oparms = (struct cifs_open_parms) { > .tcon = tcon, > .path = full_path, > @@ -596,7 +608,7 @@ int smb2_query_path_info(const unsigned int xid, > 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, data, SMB2_OP_QUERY_INFO, cfile, > - NULL, NULL, out_iov, out_buftype); > + NULL, NULL, out_iov, out_buftype, NULL); > hdr = out_iov[0].iov_base; > /* > * If first iov is unset, then SMB session was dropped or we've got a > @@ -619,7 +631,7 @@ int smb2_query_path_info(const unsigned int xid, > FILE_READ_ATTRIBUTES, FILE_OPEN, > create_options, ACL_NO_MODE, data, > SMB2_OP_QUERY_INFO, cfile, NULL, NULL, > - NULL, NULL); > + NULL, NULL, NULL); > break; > case -EREMOTE: > break; > @@ -674,7 +686,7 @@ int smb311_posix_query_path_info(const unsigned int xid, > 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, data, SMB2_OP_POSIX_QUERY_INFO, cfile, > - &sidsbuf, &sidsbuflen, out_iov, out_buftype); > + &sidsbuf, &sidsbuflen, out_iov, out_buftype, NULL); > /* > * If first iov is unset, then SMB session was dropped or we've got a > * cached open file (@cfile). > @@ -696,7 +708,7 @@ int smb311_posix_query_path_info(const unsigned int xid, > rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_READ_ATTRIBUTES, > FILE_OPEN, create_options, ACL_NO_MODE, data, > SMB2_OP_POSIX_QUERY_INFO, cfile, > - &sidsbuf, &sidsbuflen, NULL, NULL); > + &sidsbuf, &sidsbuflen, NULL, NULL, NULL); > break; > } > > @@ -735,7 +747,7 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode, > return smb2_compound_op(xid, tcon, cifs_sb, name, > FILE_WRITE_ATTRIBUTES, FILE_CREATE, > CREATE_NOT_FILE, mode, NULL, SMB2_OP_MKDIR, > - NULL, NULL, NULL, NULL, NULL); > + NULL, NULL, NULL, NULL, NULL, NULL); > } > > void > @@ -757,7 +769,7 @@ smb2_mkdir_setinfo(struct inode *inode, const char *name, > tmprc = smb2_compound_op(xid, tcon, cifs_sb, name, > FILE_WRITE_ATTRIBUTES, FILE_CREATE, > CREATE_NOT_FILE, ACL_NO_MODE, > - &data, SMB2_OP_SET_INFO, cfile, NULL, NULL, NULL, NULL); > + &data, SMB2_OP_SET_INFO, cfile, NULL, NULL, NULL, NULL, NULL); > if (tmprc == 0) > cifs_i->cifsAttrs = dosattrs; > } > @@ -769,23 +781,24 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > drop_cached_dir_by_name(xid, tcon, name, cifs_sb); > return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN, > CREATE_NOT_FILE, ACL_NO_MODE, > - NULL, SMB2_OP_RMDIR, NULL, NULL, NULL, NULL, NULL); > + NULL, SMB2_OP_RMDIR, NULL, NULL, NULL, NULL, NULL, NULL); > } > > int > smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > - struct cifs_sb_info *cifs_sb) > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > { > 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, NULL, NULL, NULL, NULL); > + ACL_NO_MODE, NULL, SMB2_OP_DELETE, NULL, NULL, > + NULL, NULL, NULL, dentry); > } > > static int > smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > struct cifs_sb_info *cifs_sb, __u32 access, int command, > - struct cifsFileInfo *cfile) > + struct cifsFileInfo *cfile, struct dentry *dentry) > { > __le16 *smb2_to_name = NULL; > int rc; > @@ -797,7 +810,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > } > rc = smb2_compound_op(xid, tcon, cifs_sb, from_name, access, > FILE_OPEN, 0, ACL_NO_MODE, smb2_to_name, > - command, cfile, NULL, NULL, NULL, NULL); > + command, cfile, NULL, NULL, NULL, NULL, dentry); > smb2_rename_path: > kfree(smb2_to_name); > return rc; > @@ -806,7 +819,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > int > smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > - struct cifs_sb_info *cifs_sb) > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > { > struct cifsFileInfo *cfile; > > @@ -814,7 +827,7 @@ smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile); > > return smb2_set_path_attr(xid, tcon, from_name, to_name, > - cifs_sb, DELETE, SMB2_OP_RENAME, cfile); > + cifs_sb, DELETE, SMB2_OP_RENAME, cfile, dentry); > } > > int > @@ -824,13 +837,13 @@ smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon, > { > return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, > FILE_READ_ATTRIBUTES, SMB2_OP_HARDLINK, > - NULL); > + NULL, NULL); > } > > int > smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > const char *full_path, __u64 size, > - struct cifs_sb_info *cifs_sb, bool set_alloc) > + struct cifs_sb_info *cifs_sb, bool set_alloc, struct dentry *dentry) > { > __le64 eof = cpu_to_le64(size); > struct cifsFileInfo *cfile; > @@ -838,7 +851,7 @@ smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > 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, cfile, NULL, NULL, NULL, NULL); > + &eof, SMB2_OP_SET_EOF, cfile, NULL, NULL, NULL, NULL, dentry); > } > > int > @@ -865,7 +878,7 @@ smb2_set_file_info(struct inode *inode, const char *full_path, > rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, > FILE_WRITE_ATTRIBUTES, FILE_OPEN, > 0, ACL_NO_MODE, buf, SMB2_OP_SET_INFO, cfile, > - NULL, NULL, NULL, NULL); > + NULL, NULL, NULL, NULL, NULL); > cifs_put_tlink(tlink); > return rc; > } > diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h > index 46eff9ec302a..ec3755110da5 100644 > --- a/fs/smb/client/smb2proto.h > +++ b/fs/smb/client/smb2proto.h > @@ -62,8 +62,8 @@ int smb2_query_path_info(const unsigned int xid, > const char *full_path, > struct cifs_open_info_data *data); > extern int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > - const char *full_path, __u64 size, > - struct cifs_sb_info *cifs_sb, bool set_alloc); > + const char *full_path, __u64 size, struct cifs_sb_info *cifs_sb, > + bool set_alloc, struct dentry *dentry); > extern int smb2_set_file_info(struct inode *inode, const char *full_path, > FILE_BASIC_INFO *buf, const unsigned int xid); > extern int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, > @@ -79,10 +79,10 @@ extern void smb2_mkdir_setinfo(struct inode *inode, const char *full_path, > extern int smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, > const char *name, struct cifs_sb_info *cifs_sb); > extern int smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, > - const char *name, struct cifs_sb_info *cifs_sb); > + const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry); > extern int smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > - struct cifs_sb_info *cifs_sb); > + struct cifs_sb_info *cifs_sb, struct dentry *dentry); > extern int 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); > -- > 2.39.2 > Looks good to me. Please make sure to test this out by setting a low value for max_credits (maybe 10 or 20). Make sure that the vm layer maintains dirty buffers for longer (check vm.dirty_ratio and vm.dirty_background_ratio). And then periodically trigger renames to the file.
fixed minor whitespace error (found by scripts/checkpatch ...). also added cc: stable and Rb for Shyam Will need to update commit text Should this be split into two or three to make easier to review or is it easier to review as is? Tentatively merged into cifs-2.6.git for-next pending more testing On Sun, Dec 3, 2023 at 10:56 PM <meetakshisetiyaoss@gmail.com> wrote: > > From: Meetakshi Setiya <msetiya@microsoft.com> > > Lock contention during unlink operation causes cifs lease break ack > worker thread to block and delay sending lease break acks to server. > This case occurs when multiple threads perform unlink, write and lease > break acks on the same file. Thhis patch fixes the problem by reusing > the existing lease keys for rename, unlink and set path size compound > operations so that the client does not break its own lease. > > Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> > --- > fs/smb/client/cifsglob.h | 6 ++--- > fs/smb/client/cifsproto.h | 8 +++---- > fs/smb/client/cifssmb.c | 6 ++--- > fs/smb/client/inode.c | 12 +++++----- > fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- > fs/smb/client/smb2proto.h | 8 +++---- > 6 files changed, 51 insertions(+), 38 deletions(-) > > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h > index 7558167f603c..3f6f993357bd 100644 > --- a/fs/smb/client/cifsglob.h > +++ b/fs/smb/client/cifsglob.h > @@ -355,7 +355,7 @@ struct smb_version_operations { > struct cifs_open_info_data *data); > /* set size by path */ > int (*set_path_size)(const unsigned int, struct cifs_tcon *, > - const char *, __u64, struct cifs_sb_info *, bool); > + const char *, __u64, struct cifs_sb_info *, bool, struct dentry *); > /* set size by file handle */ > int (*set_file_size)(const unsigned int, struct cifs_tcon *, > struct cifsFileInfo *, __u64, bool); > @@ -385,13 +385,13 @@ struct smb_version_operations { > struct cifs_sb_info *); > /* unlink file */ > int (*unlink)(const unsigned int, struct cifs_tcon *, const char *, > - struct cifs_sb_info *); > + struct cifs_sb_info *, struct dentry *); > /* open, rename and delete file */ > int (*rename_pending_delete)(const char *, struct dentry *, > const unsigned int); > /* send rename request */ > int (*rename)(const unsigned int, struct cifs_tcon *, const char *, > - const char *, struct cifs_sb_info *); > + const char *, struct cifs_sb_info *, struct dentry *); > /* send create hardlink request */ > int (*create_hardlink)(const unsigned int, struct cifs_tcon *, > const char *, const char *, > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h > index 46feaa0880bd..3bb15cc74bc2 100644 > --- a/fs/smb/client/cifsproto.h > +++ b/fs/smb/client/cifsproto.h > @@ -397,8 +397,8 @@ extern int CIFSSMBSetFileDisposition(const unsigned int xid, > bool delete_file, __u16 fid, > __u32 pid_of_opener); > extern int CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, > - const char *file_name, __u64 size, > - struct cifs_sb_info *cifs_sb, bool set_allocation); > + const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, > + bool set_allocation, struct dentry *dentry); > extern int CIFSSMBSetFileSize(const unsigned int xid, struct cifs_tcon *tcon, > struct cifsFileInfo *cfile, __u64 size, > bool set_allocation); > @@ -434,10 +434,10 @@ extern int CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, > const struct nls_table *nls_codepage, > int remap_special_chars); > extern int CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, > - const char *name, struct cifs_sb_info *cifs_sb); > + const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry); > extern int CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > - struct cifs_sb_info *cifs_sb); > + struct cifs_sb_info *cifs_sb, struct dentry *dentry); > extern int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *tcon, > int netfid, const char *target_name, > const struct nls_table *nls_codepage, > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c > index 9ee348e6d106..023b3bfa7b94 100644 > --- a/fs/smb/client/cifssmb.c > +++ b/fs/smb/client/cifssmb.c > @@ -738,7 +738,7 @@ CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, > > int > CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > - struct cifs_sb_info *cifs_sb) > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > { > DELETE_FILE_REQ *pSMB = NULL; > DELETE_FILE_RSP *pSMBr = NULL; > @@ -2152,7 +2152,7 @@ CIFSSMBFlush(const unsigned int xid, struct cifs_tcon *tcon, int smb_file_id) > int > CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > - struct cifs_sb_info *cifs_sb) > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > { > int rc = 0; > RENAME_REQ *pSMB = NULL; > @@ -4982,7 +4982,7 @@ CIFSSMBQFSPosixInfo(const unsigned int xid, struct cifs_tcon *tcon, > int > CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, > const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, > - bool set_allocation) > + bool set_allocation, struct dentry *dentry) > { > struct smb_com_transaction2_spi_req *pSMB = NULL; > struct smb_com_transaction2_spi_rsp *pSMBr = NULL; > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c > index d01e9ea67ccd..d5ad54733637 100644 > --- a/fs/smb/client/inode.c > +++ b/fs/smb/client/inode.c > @@ -1828,7 +1828,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) > goto psx_del_no_retry; > } > > - rc = server->ops->unlink(xid, tcon, full_path, cifs_sb); > + rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry); > > psx_del_no_retry: > if (!rc) { > @@ -2227,7 +2227,7 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry, > return -ENOSYS; > > /* try path-based rename first */ > - rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb); > + rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb, from_dentry); > > /* > * Don't bother with rename by filehandle unless file is busy and > @@ -2774,7 +2774,7 @@ void cifs_setsize(struct inode *inode, loff_t offset) > > static int > cifs_set_file_size(struct inode *inode, struct iattr *attrs, > - unsigned int xid, const char *full_path) > + unsigned int xid, const char *full_path, struct dentry *dentry) > { > int rc; > struct cifsFileInfo *open_file; > @@ -2825,7 +2825,7 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs, > */ > if (server->ops->set_path_size) > rc = server->ops->set_path_size(xid, tcon, full_path, > - attrs->ia_size, cifs_sb, false); > + attrs->ia_size, cifs_sb, false, dentry); > else > rc = -ENOSYS; > cifs_dbg(FYI, "SetEOF by path (setattrs) rc = %d\n", rc); > @@ -2915,7 +2915,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) > rc = 0; > > if (attrs->ia_valid & ATTR_SIZE) { > - rc = cifs_set_file_size(inode, attrs, xid, full_path); > + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); > if (rc != 0) > goto out; > } > @@ -3081,7 +3081,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) > } > > if (attrs->ia_valid & ATTR_SIZE) { > - rc = cifs_set_file_size(inode, attrs, xid, full_path); > + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); > if (rc != 0) > goto cifs_setattr_exit; > } > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c > index c94940af5d4b..ebee4779c743 100644 > --- a/fs/smb/client/smb2inode.c > +++ b/fs/smb/client/smb2inode.c > @@ -48,7 +48,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > __u32 desired_access, __u32 create_disposition, __u32 create_options, > umode_t mode, void *ptr, int command, struct cifsFileInfo *cfile, > __u8 **extbuf, size_t *extbuflen, > - struct kvec *out_iov, int *out_buftype) > + struct kvec *out_iov, int *out_buftype, struct dentry *dentry) > { > struct smb2_compound_vars *vars = NULL; > struct kvec *rsp_iov; > @@ -59,6 +59,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > struct cifs_fid fid; > struct cifs_ses *ses = tcon->ses; > struct TCP_Server_Info *server; > + struct inode *inode = NULL; > + struct cifsInodeInfo *cinode = NULL; > int num_rqst = 0; > int resp_buftype[3]; > struct smb2_query_info_rsp *qi_rsp = NULL; > @@ -93,6 +95,16 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > goto finished; > } > > + //if there is an existing lease, reuse it > + if (dentry) { > + inode = d_inode(dentry); > + cinode = CIFS_I(inode); > + if (cinode->lease_granted) { > + oplock = SMB2_OPLOCK_LEVEL_LEASE; > + memcpy(fid.lease_key, cinode->lease_key, SMB2_LEASE_KEY_SIZE); > + } > + } > + > vars->oparms = (struct cifs_open_parms) { > .tcon = tcon, > .path = full_path, > @@ -596,7 +608,7 @@ int smb2_query_path_info(const unsigned int xid, > 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, data, SMB2_OP_QUERY_INFO, cfile, > - NULL, NULL, out_iov, out_buftype); > + NULL, NULL, out_iov, out_buftype, NULL); > hdr = out_iov[0].iov_base; > /* > * If first iov is unset, then SMB session was dropped or we've got a > @@ -619,7 +631,7 @@ int smb2_query_path_info(const unsigned int xid, > FILE_READ_ATTRIBUTES, FILE_OPEN, > create_options, ACL_NO_MODE, data, > SMB2_OP_QUERY_INFO, cfile, NULL, NULL, > - NULL, NULL); > + NULL, NULL, NULL); > break; > case -EREMOTE: > break; > @@ -674,7 +686,7 @@ int smb311_posix_query_path_info(const unsigned int xid, > 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, data, SMB2_OP_POSIX_QUERY_INFO, cfile, > - &sidsbuf, &sidsbuflen, out_iov, out_buftype); > + &sidsbuf, &sidsbuflen, out_iov, out_buftype, NULL); > /* > * If first iov is unset, then SMB session was dropped or we've got a > * cached open file (@cfile). > @@ -696,7 +708,7 @@ int smb311_posix_query_path_info(const unsigned int xid, > rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_READ_ATTRIBUTES, > FILE_OPEN, create_options, ACL_NO_MODE, data, > SMB2_OP_POSIX_QUERY_INFO, cfile, > - &sidsbuf, &sidsbuflen, NULL, NULL); > + &sidsbuf, &sidsbuflen, NULL, NULL, NULL); > break; > } > > @@ -735,7 +747,7 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode, > return smb2_compound_op(xid, tcon, cifs_sb, name, > FILE_WRITE_ATTRIBUTES, FILE_CREATE, > CREATE_NOT_FILE, mode, NULL, SMB2_OP_MKDIR, > - NULL, NULL, NULL, NULL, NULL); > + NULL, NULL, NULL, NULL, NULL, NULL); > } > > void > @@ -757,7 +769,7 @@ smb2_mkdir_setinfo(struct inode *inode, const char *name, > tmprc = smb2_compound_op(xid, tcon, cifs_sb, name, > FILE_WRITE_ATTRIBUTES, FILE_CREATE, > CREATE_NOT_FILE, ACL_NO_MODE, > - &data, SMB2_OP_SET_INFO, cfile, NULL, NULL, NULL, NULL); > + &data, SMB2_OP_SET_INFO, cfile, NULL, NULL, NULL, NULL, NULL); > if (tmprc == 0) > cifs_i->cifsAttrs = dosattrs; > } > @@ -769,23 +781,24 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > drop_cached_dir_by_name(xid, tcon, name, cifs_sb); > return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN, > CREATE_NOT_FILE, ACL_NO_MODE, > - NULL, SMB2_OP_RMDIR, NULL, NULL, NULL, NULL, NULL); > + NULL, SMB2_OP_RMDIR, NULL, NULL, NULL, NULL, NULL, NULL); > } > > int > smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > - struct cifs_sb_info *cifs_sb) > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > { > 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, NULL, NULL, NULL, NULL); > + ACL_NO_MODE, NULL, SMB2_OP_DELETE, NULL, NULL, > + NULL, NULL, NULL, dentry); > } > > static int > smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > struct cifs_sb_info *cifs_sb, __u32 access, int command, > - struct cifsFileInfo *cfile) > + struct cifsFileInfo *cfile, struct dentry *dentry) > { > __le16 *smb2_to_name = NULL; > int rc; > @@ -797,7 +810,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > } > rc = smb2_compound_op(xid, tcon, cifs_sb, from_name, access, > FILE_OPEN, 0, ACL_NO_MODE, smb2_to_name, > - command, cfile, NULL, NULL, NULL, NULL); > + command, cfile, NULL, NULL, NULL, NULL, dentry); > smb2_rename_path: > kfree(smb2_to_name); > return rc; > @@ -806,7 +819,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > int > smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > - struct cifs_sb_info *cifs_sb) > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > { > struct cifsFileInfo *cfile; > > @@ -814,7 +827,7 @@ smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile); > > return smb2_set_path_attr(xid, tcon, from_name, to_name, > - cifs_sb, DELETE, SMB2_OP_RENAME, cfile); > + cifs_sb, DELETE, SMB2_OP_RENAME, cfile, dentry); > } > > int > @@ -824,13 +837,13 @@ smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon, > { > return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, > FILE_READ_ATTRIBUTES, SMB2_OP_HARDLINK, > - NULL); > + NULL, NULL); > } > > int > smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > const char *full_path, __u64 size, > - struct cifs_sb_info *cifs_sb, bool set_alloc) > + struct cifs_sb_info *cifs_sb, bool set_alloc, struct dentry *dentry) > { > __le64 eof = cpu_to_le64(size); > struct cifsFileInfo *cfile; > @@ -838,7 +851,7 @@ smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > 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, cfile, NULL, NULL, NULL, NULL); > + &eof, SMB2_OP_SET_EOF, cfile, NULL, NULL, NULL, NULL, dentry); > } > > int > @@ -865,7 +878,7 @@ smb2_set_file_info(struct inode *inode, const char *full_path, > rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, > FILE_WRITE_ATTRIBUTES, FILE_OPEN, > 0, ACL_NO_MODE, buf, SMB2_OP_SET_INFO, cfile, > - NULL, NULL, NULL, NULL); > + NULL, NULL, NULL, NULL, NULL); > cifs_put_tlink(tlink); > return rc; > } > diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h > index 46eff9ec302a..ec3755110da5 100644 > --- a/fs/smb/client/smb2proto.h > +++ b/fs/smb/client/smb2proto.h > @@ -62,8 +62,8 @@ int smb2_query_path_info(const unsigned int xid, > const char *full_path, > struct cifs_open_info_data *data); > extern int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > - const char *full_path, __u64 size, > - struct cifs_sb_info *cifs_sb, bool set_alloc); > + const char *full_path, __u64 size, struct cifs_sb_info *cifs_sb, > + bool set_alloc, struct dentry *dentry); > extern int smb2_set_file_info(struct inode *inode, const char *full_path, > FILE_BASIC_INFO *buf, const unsigned int xid); > extern int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, > @@ -79,10 +79,10 @@ extern void smb2_mkdir_setinfo(struct inode *inode, const char *full_path, > extern int smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, > const char *name, struct cifs_sb_info *cifs_sb); > extern int smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, > - const char *name, struct cifs_sb_info *cifs_sb); > + const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry); > extern int smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > const char *from_name, const char *to_name, > - struct cifs_sb_info *cifs_sb); > + struct cifs_sb_info *cifs_sb, struct dentry *dentry); > extern int 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); > -- > 2.39.2 >
On Tue, Dec 5, 2023 at 1:53 AM Steve French <smfrench@gmail.com> wrote: > > fixed minor whitespace error (found by scripts/checkpatch ...). also > added cc: stable and Rb for Shyam > > Will need to update commit text > > Should this be split into two or three to make easier to review or is > it easier to review as is? I think it's easy to review. Even if the number of lines of change seems large, the effective changes are relatively small. The changes to function handler prototypes led to increase in the number of lines of change. > > Tentatively merged into cifs-2.6.git for-next pending more testing > > On Sun, Dec 3, 2023 at 10:56 PM <meetakshisetiyaoss@gmail.com> wrote: > > > > From: Meetakshi Setiya <msetiya@microsoft.com> > > > > Lock contention during unlink operation causes cifs lease break ack > > worker thread to block and delay sending lease break acks to server. > > This case occurs when multiple threads perform unlink, write and lease > > break acks on the same file. Thhis patch fixes the problem by reusing > > the existing lease keys for rename, unlink and set path size compound > > operations so that the client does not break its own lease. > > > > Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> > > --- > > fs/smb/client/cifsglob.h | 6 ++--- > > fs/smb/client/cifsproto.h | 8 +++---- > > fs/smb/client/cifssmb.c | 6 ++--- > > fs/smb/client/inode.c | 12 +++++----- > > fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- > > fs/smb/client/smb2proto.h | 8 +++---- > > 6 files changed, 51 insertions(+), 38 deletions(-) > > > > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h > > index 7558167f603c..3f6f993357bd 100644 > > --- a/fs/smb/client/cifsglob.h > > +++ b/fs/smb/client/cifsglob.h > > @@ -355,7 +355,7 @@ struct smb_version_operations { > > struct cifs_open_info_data *data); > > /* set size by path */ > > int (*set_path_size)(const unsigned int, struct cifs_tcon *, > > - const char *, __u64, struct cifs_sb_info *, bool); > > + const char *, __u64, struct cifs_sb_info *, bool, struct dentry *); > > /* set size by file handle */ > > int (*set_file_size)(const unsigned int, struct cifs_tcon *, > > struct cifsFileInfo *, __u64, bool); > > @@ -385,13 +385,13 @@ struct smb_version_operations { > > struct cifs_sb_info *); > > /* unlink file */ > > int (*unlink)(const unsigned int, struct cifs_tcon *, const char *, > > - struct cifs_sb_info *); > > + struct cifs_sb_info *, struct dentry *); > > /* open, rename and delete file */ > > int (*rename_pending_delete)(const char *, struct dentry *, > > const unsigned int); > > /* send rename request */ > > int (*rename)(const unsigned int, struct cifs_tcon *, const char *, > > - const char *, struct cifs_sb_info *); > > + const char *, struct cifs_sb_info *, struct dentry *); > > /* send create hardlink request */ > > int (*create_hardlink)(const unsigned int, struct cifs_tcon *, > > const char *, const char *, > > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h > > index 46feaa0880bd..3bb15cc74bc2 100644 > > --- a/fs/smb/client/cifsproto.h > > +++ b/fs/smb/client/cifsproto.h > > @@ -397,8 +397,8 @@ extern int CIFSSMBSetFileDisposition(const unsigned int xid, > > bool delete_file, __u16 fid, > > __u32 pid_of_opener); > > extern int CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, > > - const char *file_name, __u64 size, > > - struct cifs_sb_info *cifs_sb, bool set_allocation); > > + const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, > > + bool set_allocation, struct dentry *dentry); > > extern int CIFSSMBSetFileSize(const unsigned int xid, struct cifs_tcon *tcon, > > struct cifsFileInfo *cfile, __u64 size, > > bool set_allocation); > > @@ -434,10 +434,10 @@ extern int CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, > > const struct nls_table *nls_codepage, > > int remap_special_chars); > > extern int CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, > > - const char *name, struct cifs_sb_info *cifs_sb); > > + const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry); > > extern int CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, > > const char *from_name, const char *to_name, > > - struct cifs_sb_info *cifs_sb); > > + struct cifs_sb_info *cifs_sb, struct dentry *dentry); > > extern int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *tcon, > > int netfid, const char *target_name, > > const struct nls_table *nls_codepage, > > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c > > index 9ee348e6d106..023b3bfa7b94 100644 > > --- a/fs/smb/client/cifssmb.c > > +++ b/fs/smb/client/cifssmb.c > > @@ -738,7 +738,7 @@ CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, > > > > int > > CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > > - struct cifs_sb_info *cifs_sb) > > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > > { > > DELETE_FILE_REQ *pSMB = NULL; > > DELETE_FILE_RSP *pSMBr = NULL; > > @@ -2152,7 +2152,7 @@ CIFSSMBFlush(const unsigned int xid, struct cifs_tcon *tcon, int smb_file_id) > > int > > CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, > > const char *from_name, const char *to_name, > > - struct cifs_sb_info *cifs_sb) > > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > > { > > int rc = 0; > > RENAME_REQ *pSMB = NULL; > > @@ -4982,7 +4982,7 @@ CIFSSMBQFSPosixInfo(const unsigned int xid, struct cifs_tcon *tcon, > > int > > CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, > > const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, > > - bool set_allocation) > > + bool set_allocation, struct dentry *dentry) > > { > > struct smb_com_transaction2_spi_req *pSMB = NULL; > > struct smb_com_transaction2_spi_rsp *pSMBr = NULL; > > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c > > index d01e9ea67ccd..d5ad54733637 100644 > > --- a/fs/smb/client/inode.c > > +++ b/fs/smb/client/inode.c > > @@ -1828,7 +1828,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) > > goto psx_del_no_retry; > > } > > > > - rc = server->ops->unlink(xid, tcon, full_path, cifs_sb); > > + rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry); > > > > psx_del_no_retry: > > if (!rc) { > > @@ -2227,7 +2227,7 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry, > > return -ENOSYS; > > > > /* try path-based rename first */ > > - rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb); > > + rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb, from_dentry); > > > > /* > > * Don't bother with rename by filehandle unless file is busy and > > @@ -2774,7 +2774,7 @@ void cifs_setsize(struct inode *inode, loff_t offset) > > > > static int > > cifs_set_file_size(struct inode *inode, struct iattr *attrs, > > - unsigned int xid, const char *full_path) > > + unsigned int xid, const char *full_path, struct dentry *dentry) > > { > > int rc; > > struct cifsFileInfo *open_file; > > @@ -2825,7 +2825,7 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs, > > */ > > if (server->ops->set_path_size) > > rc = server->ops->set_path_size(xid, tcon, full_path, > > - attrs->ia_size, cifs_sb, false); > > + attrs->ia_size, cifs_sb, false, dentry); > > else > > rc = -ENOSYS; > > cifs_dbg(FYI, "SetEOF by path (setattrs) rc = %d\n", rc); > > @@ -2915,7 +2915,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) > > rc = 0; > > > > if (attrs->ia_valid & ATTR_SIZE) { > > - rc = cifs_set_file_size(inode, attrs, xid, full_path); > > + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); > > if (rc != 0) > > goto out; > > } > > @@ -3081,7 +3081,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) > > } > > > > if (attrs->ia_valid & ATTR_SIZE) { > > - rc = cifs_set_file_size(inode, attrs, xid, full_path); > > + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); > > if (rc != 0) > > goto cifs_setattr_exit; > > } > > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c > > index c94940af5d4b..ebee4779c743 100644 > > --- a/fs/smb/client/smb2inode.c > > +++ b/fs/smb/client/smb2inode.c > > @@ -48,7 +48,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > __u32 desired_access, __u32 create_disposition, __u32 create_options, > > umode_t mode, void *ptr, int command, struct cifsFileInfo *cfile, > > __u8 **extbuf, size_t *extbuflen, > > - struct kvec *out_iov, int *out_buftype) > > + struct kvec *out_iov, int *out_buftype, struct dentry *dentry) > > { > > struct smb2_compound_vars *vars = NULL; > > struct kvec *rsp_iov; > > @@ -59,6 +59,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > struct cifs_fid fid; > > struct cifs_ses *ses = tcon->ses; > > struct TCP_Server_Info *server; > > + struct inode *inode = NULL; > > + struct cifsInodeInfo *cinode = NULL; > > int num_rqst = 0; > > int resp_buftype[3]; > > struct smb2_query_info_rsp *qi_rsp = NULL; > > @@ -93,6 +95,16 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, > > goto finished; > > } > > > > + //if there is an existing lease, reuse it > > + if (dentry) { > > + inode = d_inode(dentry); > > + cinode = CIFS_I(inode); > > + if (cinode->lease_granted) { > > + oplock = SMB2_OPLOCK_LEVEL_LEASE; > > + memcpy(fid.lease_key, cinode->lease_key, SMB2_LEASE_KEY_SIZE); > > + } > > + } > > + > > vars->oparms = (struct cifs_open_parms) { > > .tcon = tcon, > > .path = full_path, > > @@ -596,7 +608,7 @@ int smb2_query_path_info(const unsigned int xid, > > 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, data, SMB2_OP_QUERY_INFO, cfile, > > - NULL, NULL, out_iov, out_buftype); > > + NULL, NULL, out_iov, out_buftype, NULL); > > hdr = out_iov[0].iov_base; > > /* > > * If first iov is unset, then SMB session was dropped or we've got a > > @@ -619,7 +631,7 @@ int smb2_query_path_info(const unsigned int xid, > > FILE_READ_ATTRIBUTES, FILE_OPEN, > > create_options, ACL_NO_MODE, data, > > SMB2_OP_QUERY_INFO, cfile, NULL, NULL, > > - NULL, NULL); > > + NULL, NULL, NULL); > > break; > > case -EREMOTE: > > break; > > @@ -674,7 +686,7 @@ int smb311_posix_query_path_info(const unsigned int xid, > > 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, data, SMB2_OP_POSIX_QUERY_INFO, cfile, > > - &sidsbuf, &sidsbuflen, out_iov, out_buftype); > > + &sidsbuf, &sidsbuflen, out_iov, out_buftype, NULL); > > /* > > * If first iov is unset, then SMB session was dropped or we've got a > > * cached open file (@cfile). > > @@ -696,7 +708,7 @@ int smb311_posix_query_path_info(const unsigned int xid, > > rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_READ_ATTRIBUTES, > > FILE_OPEN, create_options, ACL_NO_MODE, data, > > SMB2_OP_POSIX_QUERY_INFO, cfile, > > - &sidsbuf, &sidsbuflen, NULL, NULL); > > + &sidsbuf, &sidsbuflen, NULL, NULL, NULL); > > break; > > } > > > > @@ -735,7 +747,7 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode, > > return smb2_compound_op(xid, tcon, cifs_sb, name, > > FILE_WRITE_ATTRIBUTES, FILE_CREATE, > > CREATE_NOT_FILE, mode, NULL, SMB2_OP_MKDIR, > > - NULL, NULL, NULL, NULL, NULL); > > + NULL, NULL, NULL, NULL, NULL, NULL); > > } > > > > void > > @@ -757,7 +769,7 @@ smb2_mkdir_setinfo(struct inode *inode, const char *name, > > tmprc = smb2_compound_op(xid, tcon, cifs_sb, name, > > FILE_WRITE_ATTRIBUTES, FILE_CREATE, > > CREATE_NOT_FILE, ACL_NO_MODE, > > - &data, SMB2_OP_SET_INFO, cfile, NULL, NULL, NULL, NULL); > > + &data, SMB2_OP_SET_INFO, cfile, NULL, NULL, NULL, NULL, NULL); > > if (tmprc == 0) > > cifs_i->cifsAttrs = dosattrs; > > } > > @@ -769,23 +781,24 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > > drop_cached_dir_by_name(xid, tcon, name, cifs_sb); > > return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN, > > CREATE_NOT_FILE, ACL_NO_MODE, > > - NULL, SMB2_OP_RMDIR, NULL, NULL, NULL, NULL, NULL); > > + NULL, SMB2_OP_RMDIR, NULL, NULL, NULL, NULL, NULL, NULL); > > } > > > > int > > smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name, > > - struct cifs_sb_info *cifs_sb) > > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > > { > > 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, NULL, NULL, NULL, NULL); > > + ACL_NO_MODE, NULL, SMB2_OP_DELETE, NULL, NULL, > > + NULL, NULL, NULL, dentry); > > } > > > > static int > > smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > > const char *from_name, const char *to_name, > > struct cifs_sb_info *cifs_sb, __u32 access, int command, > > - struct cifsFileInfo *cfile) > > + struct cifsFileInfo *cfile, struct dentry *dentry) > > { > > __le16 *smb2_to_name = NULL; > > int rc; > > @@ -797,7 +810,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > > } > > rc = smb2_compound_op(xid, tcon, cifs_sb, from_name, access, > > FILE_OPEN, 0, ACL_NO_MODE, smb2_to_name, > > - command, cfile, NULL, NULL, NULL, NULL); > > + command, cfile, NULL, NULL, NULL, NULL, dentry); > > smb2_rename_path: > > kfree(smb2_to_name); > > return rc; > > @@ -806,7 +819,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, > > int > > smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > > const char *from_name, const char *to_name, > > - struct cifs_sb_info *cifs_sb) > > + struct cifs_sb_info *cifs_sb, struct dentry *dentry) > > { > > struct cifsFileInfo *cfile; > > > > @@ -814,7 +827,7 @@ smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > > cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile); > > > > return smb2_set_path_attr(xid, tcon, from_name, to_name, > > - cifs_sb, DELETE, SMB2_OP_RENAME, cfile); > > + cifs_sb, DELETE, SMB2_OP_RENAME, cfile, dentry); > > } > > > > int > > @@ -824,13 +837,13 @@ smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon, > > { > > return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, > > FILE_READ_ATTRIBUTES, SMB2_OP_HARDLINK, > > - NULL); > > + NULL, NULL); > > } > > > > int > > smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > > const char *full_path, __u64 size, > > - struct cifs_sb_info *cifs_sb, bool set_alloc) > > + struct cifs_sb_info *cifs_sb, bool set_alloc, struct dentry *dentry) > > { > > __le64 eof = cpu_to_le64(size); > > struct cifsFileInfo *cfile; > > @@ -838,7 +851,7 @@ smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > > 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, cfile, NULL, NULL, NULL, NULL); > > + &eof, SMB2_OP_SET_EOF, cfile, NULL, NULL, NULL, NULL, dentry); > > } > > > > int > > @@ -865,7 +878,7 @@ smb2_set_file_info(struct inode *inode, const char *full_path, > > rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, > > FILE_WRITE_ATTRIBUTES, FILE_OPEN, > > 0, ACL_NO_MODE, buf, SMB2_OP_SET_INFO, cfile, > > - NULL, NULL, NULL, NULL); > > + NULL, NULL, NULL, NULL, NULL); > > cifs_put_tlink(tlink); > > return rc; > > } > > diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h > > index 46eff9ec302a..ec3755110da5 100644 > > --- a/fs/smb/client/smb2proto.h > > +++ b/fs/smb/client/smb2proto.h > > @@ -62,8 +62,8 @@ int smb2_query_path_info(const unsigned int xid, > > const char *full_path, > > struct cifs_open_info_data *data); > > extern int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > > - const char *full_path, __u64 size, > > - struct cifs_sb_info *cifs_sb, bool set_alloc); > > + const char *full_path, __u64 size, struct cifs_sb_info *cifs_sb, > > + bool set_alloc, struct dentry *dentry); > > extern int smb2_set_file_info(struct inode *inode, const char *full_path, > > FILE_BASIC_INFO *buf, const unsigned int xid); > > extern int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, > > @@ -79,10 +79,10 @@ extern void smb2_mkdir_setinfo(struct inode *inode, const char *full_path, > > extern int smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, > > const char *name, struct cifs_sb_info *cifs_sb); > > extern int smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, > > - const char *name, struct cifs_sb_info *cifs_sb); > > + const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry); > > extern int smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, > > const char *from_name, const char *to_name, > > - struct cifs_sb_info *cifs_sb); > > + struct cifs_sb_info *cifs_sb, struct dentry *dentry); > > extern int 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); > > -- > > 2.39.2 > > > > > -- > Thanks, > > Steve
meetakshisetiyaoss@gmail.com writes: > From: Meetakshi Setiya <msetiya@microsoft.com> > > Lock contention during unlink operation causes cifs lease break ack > worker thread to block and delay sending lease break acks to server. > This case occurs when multiple threads perform unlink, write and lease > break acks on the same file. Thhis patch fixes the problem by reusing > the existing lease keys for rename, unlink and set path size compound > operations so that the client does not break its own lease. > > Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> > --- > fs/smb/client/cifsglob.h | 6 ++--- > fs/smb/client/cifsproto.h | 8 +++---- > fs/smb/client/cifssmb.c | 6 ++--- > fs/smb/client/inode.c | 12 +++++----- > fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- > fs/smb/client/smb2proto.h | 8 +++---- > 6 files changed, 51 insertions(+), 38 deletions(-) NAK. This patch broke some xfstests. Consider this reproducer: $ cat repro.sh #!/bin/sh umount /mnt/1 &>/dev/null mount.cifs //srv/share /mnt/1 -o ...,vers=3.1.1 rm /mnt/1/* &>/dev/null pushd /mnt/1 >/dev/null touch foo ln -v foo bar rm -v bar popd >/dev/null umount /mnt/1 &>/dev/null $ ./repro.sh 'bar' => 'foo' rm: cannot remove 'bar': Invalid argument This is what going on - client creates 'foo' with RHW lease granted. - client creates hardlink file 'bar'. At this point, we have two positive dentries (foo & bar) which share same inode. - The client then attempts to remove 'bar' by re-using lease key from 'foo' through compound request CREATE(DELETE)+CLOSE, which fails with STATUS_INVALID_PARAMETER.
On Wed, Dec 6, 2023 at 1:02 AM Paulo Alcantara <pc@manguebit.com> wrote: > > meetakshisetiyaoss@gmail.com writes: > > > From: Meetakshi Setiya <msetiya@microsoft.com> > > > > Lock contention during unlink operation causes cifs lease break ack > > worker thread to block and delay sending lease break acks to server. > > This case occurs when multiple threads perform unlink, write and lease > > break acks on the same file. Thhis patch fixes the problem by reusing > > the existing lease keys for rename, unlink and set path size compound > > operations so that the client does not break its own lease. > > > > Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> > > --- > > fs/smb/client/cifsglob.h | 6 ++--- > > fs/smb/client/cifsproto.h | 8 +++---- > > fs/smb/client/cifssmb.c | 6 ++--- > > fs/smb/client/inode.c | 12 +++++----- > > fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- > > fs/smb/client/smb2proto.h | 8 +++---- > > 6 files changed, 51 insertions(+), 38 deletions(-) > > NAK. This patch broke some xfstests. > > Consider this reproducer: > > $ cat repro.sh > #!/bin/sh > > umount /mnt/1 &>/dev/null > mount.cifs //srv/share /mnt/1 -o ...,vers=3.1.1 > rm /mnt/1/* &>/dev/null > pushd /mnt/1 >/dev/null > touch foo > ln -v foo bar > rm -v bar > popd >/dev/null > umount /mnt/1 &>/dev/null > $ ./repro.sh > 'bar' => 'foo' > rm: cannot remove 'bar': Invalid argument > > This is what going on > > - client creates 'foo' with RHW lease granted. > - client creates hardlink file 'bar'. > > At this point, we have two positive dentries (foo & bar) which share > same inode. > > - The client then attempts to remove 'bar' by re-using lease key from > 'foo' through compound request CREATE(DELETE)+CLOSE, which fails with > STATUS_INVALID_PARAMETER. That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is due to the server not recognizing the lease id for the file bar. I'm not sure that this is a client bug. If the server supports hard links, then should it not be aware that foo and bar are the same files? AFAIK, file lease is associated with a file, and not the dentry. Meetakshi, can you please follow the repro steps provided by Paulo (against Windows file server) and check why the invalid parameter error is being returned?
I will check and get back to you. --- Thanks Meetakshi On Wed, Dec 6, 2023 at 11:02 AM Shyam Prasad N <nspmangalore@gmail.com> wrote: > > On Wed, Dec 6, 2023 at 1:02 AM Paulo Alcantara <pc@manguebit.com> wrote: > > > > meetakshisetiyaoss@gmail.com writes: > > > > > From: Meetakshi Setiya <msetiya@microsoft.com> > > > > > > Lock contention during unlink operation causes cifs lease break ack > > > worker thread to block and delay sending lease break acks to server. > > > This case occurs when multiple threads perform unlink, write and lease > > > break acks on the same file. Thhis patch fixes the problem by reusing > > > the existing lease keys for rename, unlink and set path size compound > > > operations so that the client does not break its own lease. > > > > > > Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> > > > --- > > > fs/smb/client/cifsglob.h | 6 ++--- > > > fs/smb/client/cifsproto.h | 8 +++---- > > > fs/smb/client/cifssmb.c | 6 ++--- > > > fs/smb/client/inode.c | 12 +++++----- > > > fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- > > > fs/smb/client/smb2proto.h | 8 +++---- > > > 6 files changed, 51 insertions(+), 38 deletions(-) > > > > NAK. This patch broke some xfstests. > > > > Consider this reproducer: > > > > $ cat repro.sh > > #!/bin/sh > > > > umount /mnt/1 &>/dev/null > > mount.cifs //srv/share /mnt/1 -o ...,vers=3.1.1 > > rm /mnt/1/* &>/dev/null > > pushd /mnt/1 >/dev/null > > touch foo > > ln -v foo bar > > rm -v bar > > popd >/dev/null > > umount /mnt/1 &>/dev/null > > $ ./repro.sh > > 'bar' => 'foo' > > rm: cannot remove 'bar': Invalid argument > > > > This is what going on > > > > - client creates 'foo' with RHW lease granted. > > - client creates hardlink file 'bar'. > > > > At this point, we have two positive dentries (foo & bar) which share > > same inode. > > > > - The client then attempts to remove 'bar' by re-using lease key from > > 'foo' through compound request CREATE(DELETE)+CLOSE, which fails with > > STATUS_INVALID_PARAMETER. > > That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is > due to the server not recognizing the lease id for the file bar. > I'm not sure that this is a client bug. > If the server supports hard links, then should it not be aware that > foo and bar are the same files? AFAIK, file lease is associated with a > file, and not the dentry. > Meetakshi, can you please follow the repro steps provided by Paulo > (against Windows file server) and check why the invalid parameter > error is being returned? > > -- > Regards, > Shyam
On 12/6/2023 12:31 AM, Shyam Prasad N wrote: > On Wed, Dec 6, 2023 at 1:02 AM Paulo Alcantara <pc@manguebit.com> wrote: >> >> meetakshisetiyaoss@gmail.com writes: >> >>> From: Meetakshi Setiya <msetiya@microsoft.com> >>> >>> Lock contention during unlink operation causes cifs lease break ack >>> worker thread to block and delay sending lease break acks to server. >>> This case occurs when multiple threads perform unlink, write and lease >>> break acks on the same file. Thhis patch fixes the problem by reusing >>> the existing lease keys for rename, unlink and set path size compound >>> operations so that the client does not break its own lease. >>> >>> Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> >>> --- >>> fs/smb/client/cifsglob.h | 6 ++--- >>> fs/smb/client/cifsproto.h | 8 +++---- >>> fs/smb/client/cifssmb.c | 6 ++--- >>> fs/smb/client/inode.c | 12 +++++----- >>> fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- >>> fs/smb/client/smb2proto.h | 8 +++---- >>> 6 files changed, 51 insertions(+), 38 deletions(-) >> >> NAK. This patch broke some xfstests. >> >> Consider this reproducer: >> >> $ cat repro.sh >> #!/bin/sh >> >> umount /mnt/1 &>/dev/null >> mount.cifs //srv/share /mnt/1 -o ...,vers=3.1.1 >> rm /mnt/1/* &>/dev/null >> pushd /mnt/1 >/dev/null >> touch foo >> ln -v foo bar >> rm -v bar >> popd >/dev/null >> umount /mnt/1 &>/dev/null >> $ ./repro.sh >> 'bar' => 'foo' >> rm: cannot remove 'bar': Invalid argument >> >> This is what going on >> >> - client creates 'foo' with RHW lease granted. >> - client creates hardlink file 'bar'. >> >> At this point, we have two positive dentries (foo & bar) which share >> same inode. >> >> - The client then attempts to remove 'bar' by re-using lease key from >> 'foo' through compound request CREATE(DELETE)+CLOSE, which fails with >> STATUS_INVALID_PARAMETER. > > That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is > due to the server not recognizing the lease id for the file bar. > I'm not sure that this is a client bug. > If the server supports hard links, then should it not be aware that > foo and bar are the same files? AFAIK, file lease is associated with a > file, and not the dentry. Lease keys are tied to the _filename_, including the full path. They are basically guid's, and are used as lookup keys in the lease list, from which the lease itself is the resulting value. The value is then inspected for a match to the filename for which it was created. They are not about the _handle_, which is what is apparently being assumed here. MS-SMB2 section 3.3.5.9.8 says this, which the server in question is correctly enforcing [emphasis added]: > The server MUST attempt to locate a Lease by performing a lookup in the LeaseTable.LeaseList > using the LeaseKey in the SMB2_CREATE_REQUEST_LEASE as the lookup key. If a lease is found, > Lease.FileDeleteOnClose is FALSE, and *Lease.Filename does not match the file name* for the > incoming request, the request MUST be failed with STATUS_INVALID_PARAMETER IOW, hard links are, from a protocol leasing standpoint, not the same "file". Tom. > Meetakshi, can you please follow the repro steps provided by Paulo > (against Windows file server) and check why the invalid parameter > error is being returned? >
Shyam Prasad N <nspmangalore@gmail.com> writes: > That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is > due to the server not recognizing the lease id for the file bar. > I'm not sure that this is a client bug. > If the server supports hard links, then should it not be aware that > foo and bar are the same files? AFAIK, file lease is associated with a > file, and not the dentry. The patch is doing + //if there is an existing lease, reuse it + if (dentry) { + inode = d_inode(dentry); + cinode = CIFS_I(inode); + if (cinode->lease_granted) { + oplock = SMB2_OPLOCK_LEVEL_LEASE; + memcpy(fid.lease_key, cinode->lease_key, SMB2_LEASE_KEY_SIZE); + } + } and @inode ends up being the same for foo and bar from reproducer. So, the client is trying to close bar file by using lease key from foo. The server then fails to match @cinode->lease_key for bar file.
Heeding the conversation above, I have updated the patch and added it as an attachment here. The problem with failing xfstests with this patch was reusing the lease key for a file to do operations on its hardlink. As per some investigations I performed on the windows smb server and, in the MS-SMB2 docs, leases are associated with file (full) path like Tom mentioned. Since we maintain lease key with the inode on the client, as a temporary fix, I have added a check to avoid sending the lease key when a file has hardlinks. Thanks Meetakshi On Wed, Dec 6, 2023 at 7:38 PM Paulo Alcantara <pc@manguebit.com> wrote: > > Shyam Prasad N <nspmangalore@gmail.com> writes: > > > That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is > > due to the server not recognizing the lease id for the file bar. > > I'm not sure that this is a client bug. > > If the server supports hard links, then should it not be aware that > > foo and bar are the same files? AFAIK, file lease is associated with a > > file, and not the dentry. > > The patch is doing > > + //if there is an existing lease, reuse it > + if (dentry) { > + inode = d_inode(dentry); > + cinode = CIFS_I(inode); > + if (cinode->lease_granted) { > + oplock = SMB2_OPLOCK_LEVEL_LEASE; > + memcpy(fid.lease_key, cinode->lease_key, SMB2_LEASE_KEY_SIZE); > + } > + } > > and @inode ends up being the same for foo and bar from reproducer. So, > the client is trying to close bar file by using lease key from foo. The > server then fails to match @cinode->lease_key for bar file.
On Wed, Dec 6, 2023 at 7:12 PM Tom Talpey <tom@talpey.com> wrote: > > On 12/6/2023 12:31 AM, Shyam Prasad N wrote: > > On Wed, Dec 6, 2023 at 1:02 AM Paulo Alcantara <pc@manguebit.com> wrote: > >> > >> meetakshisetiyaoss@gmail.com writes: > >> > >>> From: Meetakshi Setiya <msetiya@microsoft.com> > >>> > >>> Lock contention during unlink operation causes cifs lease break ack > >>> worker thread to block and delay sending lease break acks to server. > >>> This case occurs when multiple threads perform unlink, write and lease > >>> break acks on the same file. Thhis patch fixes the problem by reusing > >>> the existing lease keys for rename, unlink and set path size compound > >>> operations so that the client does not break its own lease. > >>> > >>> Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> > >>> --- > >>> fs/smb/client/cifsglob.h | 6 ++--- > >>> fs/smb/client/cifsproto.h | 8 +++---- > >>> fs/smb/client/cifssmb.c | 6 ++--- > >>> fs/smb/client/inode.c | 12 +++++----- > >>> fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- > >>> fs/smb/client/smb2proto.h | 8 +++---- > >>> 6 files changed, 51 insertions(+), 38 deletions(-) > >> > >> NAK. This patch broke some xfstests. > >> > >> Consider this reproducer: > >> > >> $ cat repro.sh > >> #!/bin/sh > >> > >> umount /mnt/1 &>/dev/null > >> mount.cifs //srv/share /mnt/1 -o ...,vers=3.1.1 > >> rm /mnt/1/* &>/dev/null > >> pushd /mnt/1 >/dev/null > >> touch foo > >> ln -v foo bar > >> rm -v bar > >> popd >/dev/null > >> umount /mnt/1 &>/dev/null > >> $ ./repro.sh > >> 'bar' => 'foo' > >> rm: cannot remove 'bar': Invalid argument > >> > >> This is what going on > >> > >> - client creates 'foo' with RHW lease granted. > >> - client creates hardlink file 'bar'. > >> > >> At this point, we have two positive dentries (foo & bar) which share > >> same inode. > >> > >> - The client then attempts to remove 'bar' by re-using lease key from > >> 'foo' through compound request CREATE(DELETE)+CLOSE, which fails with > >> STATUS_INVALID_PARAMETER. > > > > That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is > > due to the server not recognizing the lease id for the file bar. > > I'm not sure that this is a client bug. > > If the server supports hard links, then should it not be aware that > > foo and bar are the same files? AFAIK, file lease is associated with a > > file, and not the dentry. > > Lease keys are tied to the _filename_, including the full path. They > are basically guid's, and are used as lookup keys in the lease list, > from which the lease itself is the resulting value. The value is then > inspected for a match to the filename for which it was created. > They are not about the _handle_, which is what is apparently being > assumed here. > > MS-SMB2 section 3.3.5.9.8 says this, which the server in question is > correctly enforcing [emphasis added]: > > > The server MUST attempt to locate a Lease by performing a lookup in the LeaseTable.LeaseList > > using the LeaseKey in the SMB2_CREATE_REQUEST_LEASE as the lookup key. If a lease is found, > > Lease.FileDeleteOnClose is FALSE, and *Lease.Filename does not match the file name* for the > > incoming request, the request MUST be failed with STATUS_INVALID_PARAMETER > > IOW, hard links are, from a protocol leasing standpoint, not the > same "file". This is interesting. I would assume that leases are a mechanism that assure client that it is free to cache the file locally, and that the server would inform the client when that is not safe anymore. Hard links are in-fact pointing to the same file. So I would've assumed that the server would have treated both links to have the same lease. So either the server should share leases between hard links. If not, at least an existing RWH lease on link1 would be broken by RWH lease requested by another client on link2. At least for the Windows file server, that does not happen either. Isn't this a bug from the correctness stand point? > > Tom. > > > > Meetakshi, can you please follow the repro steps provided by Paulo > > (against Windows file server) and check why the invalid parameter > > error is being returned? > >
On 12/8/2023 5:37 AM, Meetakshi Setiya wrote: > Heeding the conversation above, I have updated the patch and added it as > an attachment here. The problem with failing xfstests with this patch was > reusing the lease key for a file to do operations on its hardlink. As per some > investigations I performed on the windows smb server and, in the MS-SMB2 docs, > leases are associated with file (full) path like Tom mentioned. Since > we maintain > lease key with the inode on the client, as a temporary fix, I have added a > check to avoid sending the lease key when a file has hardlinks. > + /* If there is an existing lease, reuse it */ > + > + /* > + * note: files with hardlinks cause unexpected behaviour. As per MS-SMB2, > + * lease keys are associated with the file name. We are maintaining lease keys > + * with the inode on the client. If the file has hardlinks associated with it, > + * it could be possible that the lease for a file be reused for an operation > + * on the hardlink or vice versa. As a temporary fix, skip reusing the > + * lease if the file has hardlinks. > + */ > + if (dentry) { > + inode = d_inode(dentry); > + cinode = CIFS_I(inode); > + if (cinode->lease_granted && inode->i_nlink == 1) { > + oplock = SMB2_OPLOCK_LEVEL_LEASE; > + memcpy(fid.lease_key, cinode->lease_key, SMB2_LEASE_KEY_SIZE); > + } > + } > + This patch completely removes the fix for any hard linked file! How can you justify proposing a "temporary fix" for upstream? It will simply keep the old, problematic, behavior. Also, the patch description hasn't been changed, and is therefore no longer accurate. Nak, in its present form. Tom. > > Thanks > Meetakshi > > On Wed, Dec 6, 2023 at 7:38 PM Paulo Alcantara <pc@manguebit.com> wrote: >> >> Shyam Prasad N <nspmangalore@gmail.com> writes: >> >>> That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is >>> due to the server not recognizing the lease id for the file bar. >>> I'm not sure that this is a client bug. >>> If the server supports hard links, then should it not be aware that >>> foo and bar are the same files? AFAIK, file lease is associated with a >>> file, and not the dentry. >> >> The patch is doing >> >> + //if there is an existing lease, reuse it >> + if (dentry) { >> + inode = d_inode(dentry); >> + cinode = CIFS_I(inode); >> + if (cinode->lease_granted) { >> + oplock = SMB2_OPLOCK_LEVEL_LEASE; >> + memcpy(fid.lease_key, cinode->lease_key, SMB2_LEASE_KEY_SIZE); >> + } >> + } >> >> and @inode ends up being the same for foo and bar from reproducer. So, >> the client is trying to close bar file by using lease key from foo. The >> server then fails to match @cinode->lease_key for bar file.
On 12/8/2023 5:58 AM, Shyam Prasad N wrote: > On Wed, Dec 6, 2023 at 7:12 PM Tom Talpey <tom@talpey.com> wrote: >> >> On 12/6/2023 12:31 AM, Shyam Prasad N wrote: >>> On Wed, Dec 6, 2023 at 1:02 AM Paulo Alcantara <pc@manguebit.com> wrote: >>>> >>>> meetakshisetiyaoss@gmail.com writes: >>>> >>>>> From: Meetakshi Setiya <msetiya@microsoft.com> >>>>> >>>>> Lock contention during unlink operation causes cifs lease break ack >>>>> worker thread to block and delay sending lease break acks to server. >>>>> This case occurs when multiple threads perform unlink, write and lease >>>>> break acks on the same file. Thhis patch fixes the problem by reusing >>>>> the existing lease keys for rename, unlink and set path size compound >>>>> operations so that the client does not break its own lease. >>>>> >>>>> Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> >>>>> --- >>>>> fs/smb/client/cifsglob.h | 6 ++--- >>>>> fs/smb/client/cifsproto.h | 8 +++---- >>>>> fs/smb/client/cifssmb.c | 6 ++--- >>>>> fs/smb/client/inode.c | 12 +++++----- >>>>> fs/smb/client/smb2inode.c | 49 +++++++++++++++++++++++++-------------- >>>>> fs/smb/client/smb2proto.h | 8 +++---- >>>>> 6 files changed, 51 insertions(+), 38 deletions(-) >>>> >>>> NAK. This patch broke some xfstests. >>>> >>>> Consider this reproducer: >>>> >>>> $ cat repro.sh >>>> #!/bin/sh >>>> >>>> umount /mnt/1 &>/dev/null >>>> mount.cifs //srv/share /mnt/1 -o ...,vers=3.1.1 >>>> rm /mnt/1/* &>/dev/null >>>> pushd /mnt/1 >/dev/null >>>> touch foo >>>> ln -v foo bar >>>> rm -v bar >>>> popd >/dev/null >>>> umount /mnt/1 &>/dev/null >>>> $ ./repro.sh >>>> 'bar' => 'foo' >>>> rm: cannot remove 'bar': Invalid argument >>>> >>>> This is what going on >>>> >>>> - client creates 'foo' with RHW lease granted. >>>> - client creates hardlink file 'bar'. >>>> >>>> At this point, we have two positive dentries (foo & bar) which share >>>> same inode. >>>> >>>> - The client then attempts to remove 'bar' by re-using lease key from >>>> 'foo' through compound request CREATE(DELETE)+CLOSE, which fails with >>>> STATUS_INVALID_PARAMETER. >>> >>> That's interesting. I'm assuming that the STATUS_INVALID_PARAMETER is >>> due to the server not recognizing the lease id for the file bar. >>> I'm not sure that this is a client bug. >>> If the server supports hard links, then should it not be aware that >>> foo and bar are the same files? AFAIK, file lease is associated with a >>> file, and not the dentry. >> >> Lease keys are tied to the _filename_, including the full path. They >> are basically guid's, and are used as lookup keys in the lease list, >> from which the lease itself is the resulting value. The value is then >> inspected for a match to the filename for which it was created. >> They are not about the _handle_, which is what is apparently being >> assumed here. >> >> MS-SMB2 section 3.3.5.9.8 says this, which the server in question is >> correctly enforcing [emphasis added]: >> >>> The server MUST attempt to locate a Lease by performing a lookup in the LeaseTable.LeaseList >>> using the LeaseKey in the SMB2_CREATE_REQUEST_LEASE as the lookup key. If a lease is found, >>> Lease.FileDeleteOnClose is FALSE, and *Lease.Filename does not match the file name* for the >>> incoming request, the request MUST be failed with STATUS_INVALID_PARAMETER >> >> IOW, hard links are, from a protocol leasing standpoint, not the >> same "file". > > This is interesting. I would assume that leases are a mechanism that > assure client that it is free to cache the file locally, and that the > server would inform the client when that is not safe anymore. MS-SMB2 has this (non-normative) statement in section 3.2.4.3: > If the client accesses a file through multiple paths, such as using > different server names or share names or parent directory names, it > will create multiple File elements, and therefore multiple > File.LeaseKeys for the same remote file. This loses the performance > benefits of sharing cache state across all Opens of the same file and > can cause additional lease breaks to be generated, as actions by a > client through one path will affect caching by that client through > other paths. However, the impact is a matter of performance; cache > correctness is preserved. If the client accesses the same path > across multiple opens, the client will use the same File element and > therefore the same File.LeaseKey is used. So, this next statement makes an incorrect assumption: > Hard links are in-fact pointing to the same file. So I would've > assumed that the server would have treated both links to have the same > lease. Not at all - the lease key is chosen by the *client*, not the server, and the key can only look up one actual lease. So the protocol enforces it, meaning the server must verify the path. > So either the server should share leases between hard links. > If not, at least an existing RWH lease on link1 would be broken by RWH > lease requested by another client on link2. At least for the Windows > file server, that does not happen either. > Isn't this a bug from the correctness stand point? Can you provide traces on that? If the protocol document is incorrect, or the Windows implementation does not conform to it, that's bad. Tom. > >> >> Tom. >> >> >>> Meetakshi, can you please follow the repro steps provided by Paulo >>> (against Windows file server) and check why the invalid parameter >>> error is being returned? >>> > > >
On Fri, Dec 08, 2023 at 09:02:45AM -0500, Tom Talpey wrote: >So, this next statement makes an incorrect assumption: > >>Hard links are in-fact pointing to the same file. So I would've >>assumed that the server would have treated both links to have the same >>lease. > >Not at all - the lease key is chosen by the *client*, not the server, >and the key can only look up one actual lease. So the protocol enforces >it, meaning the server must verify the path. IMHO this was a big mistake in the protocol design. The server has complete knowledge of what paths map to what files, so the lease key really should have been a server created token. Well, too late now :-).
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 7558167f603c..3f6f993357bd 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -355,7 +355,7 @@ struct smb_version_operations { struct cifs_open_info_data *data); /* set size by path */ int (*set_path_size)(const unsigned int, struct cifs_tcon *, - const char *, __u64, struct cifs_sb_info *, bool); + const char *, __u64, struct cifs_sb_info *, bool, struct dentry *); /* set size by file handle */ int (*set_file_size)(const unsigned int, struct cifs_tcon *, struct cifsFileInfo *, __u64, bool); @@ -385,13 +385,13 @@ struct smb_version_operations { struct cifs_sb_info *); /* unlink file */ int (*unlink)(const unsigned int, struct cifs_tcon *, const char *, - struct cifs_sb_info *); + struct cifs_sb_info *, struct dentry *); /* open, rename and delete file */ int (*rename_pending_delete)(const char *, struct dentry *, const unsigned int); /* send rename request */ int (*rename)(const unsigned int, struct cifs_tcon *, const char *, - const char *, struct cifs_sb_info *); + const char *, struct cifs_sb_info *, struct dentry *); /* send create hardlink request */ int (*create_hardlink)(const unsigned int, struct cifs_tcon *, const char *, const char *, diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index 46feaa0880bd..3bb15cc74bc2 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -397,8 +397,8 @@ extern int CIFSSMBSetFileDisposition(const unsigned int xid, bool delete_file, __u16 fid, __u32 pid_of_opener); extern int CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, - const char *file_name, __u64 size, - struct cifs_sb_info *cifs_sb, bool set_allocation); + const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, + bool set_allocation, struct dentry *dentry); extern int CIFSSMBSetFileSize(const unsigned int xid, struct cifs_tcon *tcon, struct cifsFileInfo *cfile, __u64 size, bool set_allocation); @@ -434,10 +434,10 @@ extern int CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, const struct nls_table *nls_codepage, int remap_special_chars); extern int CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, - const char *name, struct cifs_sb_info *cifs_sb); + const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry); extern int CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, const char *from_name, const char *to_name, - struct cifs_sb_info *cifs_sb); + struct cifs_sb_info *cifs_sb, struct dentry *dentry); extern int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *tcon, int netfid, const char *target_name, const struct nls_table *nls_codepage, diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c index 9ee348e6d106..023b3bfa7b94 100644 --- a/fs/smb/client/cifssmb.c +++ b/fs/smb/client/cifssmb.c @@ -738,7 +738,7 @@ CIFSPOSIXDelFile(const unsigned int xid, struct cifs_tcon *tcon, int CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, const char *name, - struct cifs_sb_info *cifs_sb) + struct cifs_sb_info *cifs_sb, struct dentry *dentry) { DELETE_FILE_REQ *pSMB = NULL; DELETE_FILE_RSP *pSMBr = NULL; @@ -2152,7 +2152,7 @@ CIFSSMBFlush(const unsigned int xid, struct cifs_tcon *tcon, int smb_file_id) int CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon, const char *from_name, const char *to_name, - struct cifs_sb_info *cifs_sb) + struct cifs_sb_info *cifs_sb, struct dentry *dentry) { int rc = 0; RENAME_REQ *pSMB = NULL; @@ -4982,7 +4982,7 @@ CIFSSMBQFSPosixInfo(const unsigned int xid, struct cifs_tcon *tcon, int CIFSSMBSetEOF(const unsigned int xid, struct cifs_tcon *tcon, const char *file_name, __u64 size, struct cifs_sb_info *cifs_sb, - bool set_allocation) + bool set_allocation, struct dentry *dentry) { struct smb_com_transaction2_spi_req *pSMB = NULL; struct smb_com_transaction2_spi_rsp *pSMBr = NULL; diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index d01e9ea67ccd..d5ad54733637 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -1828,7 +1828,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) goto psx_del_no_retry; } - rc = server->ops->unlink(xid, tcon, full_path, cifs_sb); + rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry); psx_del_no_retry: if (!rc) { @@ -2227,7 +2227,7 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry, return -ENOSYS; /* try path-based rename first */ - rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb); + rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb, from_dentry); /* * Don't bother with rename by filehandle unless file is busy and @@ -2774,7 +2774,7 @@ void cifs_setsize(struct inode *inode, loff_t offset) static int cifs_set_file_size(struct inode *inode, struct iattr *attrs, - unsigned int xid, const char *full_path) + unsigned int xid, const char *full_path, struct dentry *dentry) { int rc; struct cifsFileInfo *open_file; @@ -2825,7 +2825,7 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs, */ if (server->ops->set_path_size) rc = server->ops->set_path_size(xid, tcon, full_path, - attrs->ia_size, cifs_sb, false); + attrs->ia_size, cifs_sb, false, dentry); else rc = -ENOSYS; cifs_dbg(FYI, "SetEOF by path (setattrs) rc = %d\n", rc); @@ -2915,7 +2915,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) rc = 0; if (attrs->ia_valid & ATTR_SIZE) { - rc = cifs_set_file_size(inode, attrs, xid, full_path); + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); if (rc != 0) goto out; } @@ -3081,7 +3081,7 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) } if (attrs->ia_valid & ATTR_SIZE) { - rc = cifs_set_file_size(inode, attrs, xid, full_path); + rc = cifs_set_file_size(inode, attrs, xid, full_path, direntry); if (rc != 0) goto cifs_setattr_exit; } diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index c94940af5d4b..ebee4779c743 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -48,7 +48,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, __u32 desired_access, __u32 create_disposition, __u32 create_options, umode_t mode, void *ptr, int command, struct cifsFileInfo *cfile, __u8 **extbuf, size_t *extbuflen, - struct kvec *out_iov, int *out_buftype) + struct kvec *out_iov, int *out_buftype, struct dentry *dentry) { struct smb2_compound_vars *vars = NULL; struct kvec *rsp_iov; @@ -59,6 +59,8 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid fid; struct cifs_ses *ses = tcon->ses; struct TCP_Server_Info *server; + struct inode *inode = NULL; + struct cifsInodeInfo *cinode = NULL; int num_rqst = 0; int resp_buftype[3]; struct smb2_query_info_rsp *qi_rsp = NULL; @@ -93,6 +95,16 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon, goto finished; } + //if there is an existing lease, reuse it + if (dentry) { + inode = d_inode(dentry); + cinode = CIFS_I(inode); + if (cinode->lease_granted) { + oplock = SMB2_OPLOCK_LEVEL_LEASE; + memcpy(fid.lease_key, cinode->lease_key, SMB2_LEASE_KEY_SIZE); + } + } + vars->oparms = (struct cifs_open_parms) { .tcon = tcon, .path = full_path, @@ -596,7 +608,7 @@ int smb2_query_path_info(const unsigned int xid, 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, data, SMB2_OP_QUERY_INFO, cfile, - NULL, NULL, out_iov, out_buftype); + NULL, NULL, out_iov, out_buftype, NULL); hdr = out_iov[0].iov_base; /* * If first iov is unset, then SMB session was dropped or we've got a @@ -619,7 +631,7 @@ int smb2_query_path_info(const unsigned int xid, FILE_READ_ATTRIBUTES, FILE_OPEN, create_options, ACL_NO_MODE, data, SMB2_OP_QUERY_INFO, cfile, NULL, NULL, - NULL, NULL); + NULL, NULL, NULL); break; case -EREMOTE: break; @@ -674,7 +686,7 @@ int smb311_posix_query_path_info(const unsigned int xid, 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, data, SMB2_OP_POSIX_QUERY_INFO, cfile, - &sidsbuf, &sidsbuflen, out_iov, out_buftype); + &sidsbuf, &sidsbuflen, out_iov, out_buftype, NULL); /* * If first iov is unset, then SMB session was dropped or we've got a * cached open file (@cfile). @@ -696,7 +708,7 @@ int smb311_posix_query_path_info(const unsigned int xid, rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_READ_ATTRIBUTES, FILE_OPEN, create_options, ACL_NO_MODE, data, SMB2_OP_POSIX_QUERY_INFO, cfile, - &sidsbuf, &sidsbuflen, NULL, NULL); + &sidsbuf, &sidsbuflen, NULL, NULL, NULL); break; } @@ -735,7 +747,7 @@ smb2_mkdir(const unsigned int xid, struct inode *parent_inode, umode_t mode, return smb2_compound_op(xid, tcon, cifs_sb, name, FILE_WRITE_ATTRIBUTES, FILE_CREATE, CREATE_NOT_FILE, mode, NULL, SMB2_OP_MKDIR, - NULL, NULL, NULL, NULL, NULL); + NULL, NULL, NULL, NULL, NULL, NULL); } void @@ -757,7 +769,7 @@ smb2_mkdir_setinfo(struct inode *inode, const char *name, tmprc = smb2_compound_op(xid, tcon, cifs_sb, name, FILE_WRITE_ATTRIBUTES, FILE_CREATE, CREATE_NOT_FILE, ACL_NO_MODE, - &data, SMB2_OP_SET_INFO, cfile, NULL, NULL, NULL, NULL); + &data, SMB2_OP_SET_INFO, cfile, NULL, NULL, NULL, NULL, NULL); if (tmprc == 0) cifs_i->cifsAttrs = dosattrs; } @@ -769,23 +781,24 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name, drop_cached_dir_by_name(xid, tcon, name, cifs_sb); return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN, CREATE_NOT_FILE, ACL_NO_MODE, - NULL, SMB2_OP_RMDIR, NULL, NULL, NULL, NULL, NULL); + NULL, SMB2_OP_RMDIR, NULL, NULL, NULL, NULL, NULL, NULL); } int smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name, - struct cifs_sb_info *cifs_sb) + struct cifs_sb_info *cifs_sb, struct dentry *dentry) { 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, NULL, NULL, NULL, NULL); + ACL_NO_MODE, NULL, SMB2_OP_DELETE, NULL, NULL, + NULL, NULL, NULL, dentry); } static int smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, const char *from_name, const char *to_name, struct cifs_sb_info *cifs_sb, __u32 access, int command, - struct cifsFileInfo *cfile) + struct cifsFileInfo *cfile, struct dentry *dentry) { __le16 *smb2_to_name = NULL; int rc; @@ -797,7 +810,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, } rc = smb2_compound_op(xid, tcon, cifs_sb, from_name, access, FILE_OPEN, 0, ACL_NO_MODE, smb2_to_name, - command, cfile, NULL, NULL, NULL, NULL); + command, cfile, NULL, NULL, NULL, NULL, dentry); smb2_rename_path: kfree(smb2_to_name); return rc; @@ -806,7 +819,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon, int smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, const char *from_name, const char *to_name, - struct cifs_sb_info *cifs_sb) + struct cifs_sb_info *cifs_sb, struct dentry *dentry) { struct cifsFileInfo *cfile; @@ -814,7 +827,7 @@ smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile); return smb2_set_path_attr(xid, tcon, from_name, to_name, - cifs_sb, DELETE, SMB2_OP_RENAME, cfile); + cifs_sb, DELETE, SMB2_OP_RENAME, cfile, dentry); } int @@ -824,13 +837,13 @@ smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon, { return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb, FILE_READ_ATTRIBUTES, SMB2_OP_HARDLINK, - NULL); + NULL, NULL); } int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, const char *full_path, __u64 size, - struct cifs_sb_info *cifs_sb, bool set_alloc) + struct cifs_sb_info *cifs_sb, bool set_alloc, struct dentry *dentry) { __le64 eof = cpu_to_le64(size); struct cifsFileInfo *cfile; @@ -838,7 +851,7 @@ smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, 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, cfile, NULL, NULL, NULL, NULL); + &eof, SMB2_OP_SET_EOF, cfile, NULL, NULL, NULL, NULL, dentry); } int @@ -865,7 +878,7 @@ smb2_set_file_info(struct inode *inode, const char *full_path, rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, ACL_NO_MODE, buf, SMB2_OP_SET_INFO, cfile, - NULL, NULL, NULL, NULL); + NULL, NULL, NULL, NULL, NULL); cifs_put_tlink(tlink); return rc; } diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h index 46eff9ec302a..ec3755110da5 100644 --- a/fs/smb/client/smb2proto.h +++ b/fs/smb/client/smb2proto.h @@ -62,8 +62,8 @@ int smb2_query_path_info(const unsigned int xid, const char *full_path, struct cifs_open_info_data *data); extern int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, - const char *full_path, __u64 size, - struct cifs_sb_info *cifs_sb, bool set_alloc); + const char *full_path, __u64 size, struct cifs_sb_info *cifs_sb, + bool set_alloc, struct dentry *dentry); extern int smb2_set_file_info(struct inode *inode, const char *full_path, FILE_BASIC_INFO *buf, const unsigned int xid); extern int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, @@ -79,10 +79,10 @@ extern void smb2_mkdir_setinfo(struct inode *inode, const char *full_path, extern int smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name, struct cifs_sb_info *cifs_sb); extern int smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, - const char *name, struct cifs_sb_info *cifs_sb); + const char *name, struct cifs_sb_info *cifs_sb, struct dentry *dentry); extern int smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon, const char *from_name, const char *to_name, - struct cifs_sb_info *cifs_sb); + struct cifs_sb_info *cifs_sb, struct dentry *dentry); extern int 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);