| Message ID | 20210308230735.337-2-lsahlber@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] cifs: move the check for nohandlecache into open_shroot | expand |
Is there an updated version of this 9 patch series or is this current? Any additional review feedback on it? On Mon, Mar 8, 2021 at 5:08 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote: > > instead of doing it in the callsites for open_shroot. > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/smb2inode.c | 24 +++++++++++------------- > fs/cifs/smb2ops.c | 16 ++++++++-------- > 2 files changed, 19 insertions(+), 21 deletions(-) > > diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c > index 1f900b81c34a..3d59614cbe8f 100644 > --- a/fs/cifs/smb2inode.c > +++ b/fs/cifs/smb2inode.c > @@ -511,7 +511,6 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, > int rc; > struct smb2_file_all_info *smb2_data; > __u32 create_options = 0; > - bool no_cached_open = tcon->nohandlecache; > struct cifsFileInfo *cfile; > struct cached_fid *cfid = NULL; > > @@ -524,23 +523,22 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, > return -ENOMEM; > > /* If it is a root and its handle is cached then use it */ > - if (!strlen(full_path) && !no_cached_open) { > + if (!strlen(full_path)) { > rc = open_shroot(xid, tcon, cifs_sb, &cfid); > - if (rc) > - goto out; > - > - if (tcon->crfid.file_all_info_is_valid) { > - move_smb2_info_to_cifs(data, > + if (!rc) { > + if (tcon->crfid.file_all_info_is_valid) { > + move_smb2_info_to_cifs(data, > &tcon->crfid.file_all_info); > - } else { > - rc = SMB2_query_info(xid, tcon, > + } else { > + rc = SMB2_query_info(xid, tcon, > cfid->fid->persistent_fid, > cfid->fid->volatile_fid, smb2_data); > - if (!rc) > - move_smb2_info_to_cifs(data, smb2_data); > + if (!rc) > + move_smb2_info_to_cifs(data, smb2_data); > + } > + close_shroot(cfid); > + goto out; > } > - close_shroot(cfid); > - goto out; > } > > cifs_get_readable_path(tcon, full_path, &cfile); > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index f5087295424c..7ee6926153b8 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -746,6 +746,9 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, > u8 oplock = SMB2_OPLOCK_LEVEL_II; > struct cifs_fid *pfid; > > + if (tcon->nohandlecache) > + return -ENOTSUPP; > + > mutex_lock(&tcon->crfid.fid_mutex); > if (tcon->crfid.is_valid) { > cifs_dbg(FYI, "found a cached root file handle\n"); > @@ -914,7 +917,6 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, > u8 oplock = SMB2_OPLOCK_LEVEL_NONE; > struct cifs_open_parms oparms; > struct cifs_fid fid; > - bool no_cached_open = tcon->nohandlecache; > struct cached_fid *cfid = NULL; > > oparms.tcon = tcon; > @@ -924,14 +926,12 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, > oparms.fid = &fid; > oparms.reconnect = false; > > - if (no_cached_open) { > + rc = open_shroot(xid, tcon, cifs_sb, &cfid); > + if (rc == 0) > + memcpy(&fid, cfid->fid, sizeof(struct cifs_fid)); > + else > rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL, > NULL, NULL); > - } else { > - rc = open_shroot(xid, tcon, cifs_sb, &cfid); > - if (rc == 0) > - memcpy(&fid, cfid->fid, sizeof(struct cifs_fid)); > - } > if (rc) > return; > > @@ -945,7 +945,7 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, > FS_VOLUME_INFORMATION); > SMB2_QFS_attr(xid, tcon, fid.persistent_fid, fid.volatile_fid, > FS_SECTOR_SIZE_INFORMATION); /* SMB3 specific */ > - if (no_cached_open) > + if (cfid == NULL) > SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid); > else > close_shroot(cfid); > -- > 2.13.6 >
checkpatch complains about WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP #71: FILE: fs/cifs/smb2ops.c:750: include/linux/errno.h indicates that these in range 512 through 530 should never be returned to userspace: /* * These should never be seen by user programs. To return one of ERESTART* * codes, signal_pending() MUST be set. Note that ptrace can observe these * at syscall exit tracing, but they will never be left for the debugged user * process to see. */ ... but since this is not returned to userspace, perhaps it is harmless. Thoughts? On Mon, Mar 8, 2021 at 5:08 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote: > > instead of doing it in the callsites for open_shroot. > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/smb2inode.c | 24 +++++++++++------------- > fs/cifs/smb2ops.c | 16 ++++++++-------- > 2 files changed, 19 insertions(+), 21 deletions(-) > > diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c > index 1f900b81c34a..3d59614cbe8f 100644 > --- a/fs/cifs/smb2inode.c > +++ b/fs/cifs/smb2inode.c > @@ -511,7 +511,6 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, > int rc; > struct smb2_file_all_info *smb2_data; > __u32 create_options = 0; > - bool no_cached_open = tcon->nohandlecache; > struct cifsFileInfo *cfile; > struct cached_fid *cfid = NULL; > > @@ -524,23 +523,22 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, > return -ENOMEM; > > /* If it is a root and its handle is cached then use it */ > - if (!strlen(full_path) && !no_cached_open) { > + if (!strlen(full_path)) { > rc = open_shroot(xid, tcon, cifs_sb, &cfid); > - if (rc) > - goto out; > - > - if (tcon->crfid.file_all_info_is_valid) { > - move_smb2_info_to_cifs(data, > + if (!rc) { > + if (tcon->crfid.file_all_info_is_valid) { > + move_smb2_info_to_cifs(data, > &tcon->crfid.file_all_info); > - } else { > - rc = SMB2_query_info(xid, tcon, > + } else { > + rc = SMB2_query_info(xid, tcon, > cfid->fid->persistent_fid, > cfid->fid->volatile_fid, smb2_data); > - if (!rc) > - move_smb2_info_to_cifs(data, smb2_data); > + if (!rc) > + move_smb2_info_to_cifs(data, smb2_data); > + } > + close_shroot(cfid); > + goto out; > } > - close_shroot(cfid); > - goto out; > } > > cifs_get_readable_path(tcon, full_path, &cfile); > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index f5087295424c..7ee6926153b8 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -746,6 +746,9 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, > u8 oplock = SMB2_OPLOCK_LEVEL_II; > struct cifs_fid *pfid; > > + if (tcon->nohandlecache) > + return -ENOTSUPP; > + > mutex_lock(&tcon->crfid.fid_mutex); > if (tcon->crfid.is_valid) { > cifs_dbg(FYI, "found a cached root file handle\n"); > @@ -914,7 +917,6 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, > u8 oplock = SMB2_OPLOCK_LEVEL_NONE; > struct cifs_open_parms oparms; > struct cifs_fid fid; > - bool no_cached_open = tcon->nohandlecache; > struct cached_fid *cfid = NULL; > > oparms.tcon = tcon; > @@ -924,14 +926,12 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, > oparms.fid = &fid; > oparms.reconnect = false; > > - if (no_cached_open) { > + rc = open_shroot(xid, tcon, cifs_sb, &cfid); > + if (rc == 0) > + memcpy(&fid, cfid->fid, sizeof(struct cifs_fid)); > + else > rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL, > NULL, NULL); > - } else { > - rc = open_shroot(xid, tcon, cifs_sb, &cfid); > - if (rc == 0) > - memcpy(&fid, cfid->fid, sizeof(struct cifs_fid)); > - } > if (rc) > return; > > @@ -945,7 +945,7 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, > FS_VOLUME_INFORMATION); > SMB2_QFS_attr(xid, tcon, fid.persistent_fid, fid.volatile_fid, > FS_SECTOR_SIZE_INFORMATION); /* SMB3 specific */ > - if (no_cached_open) > + if (cfid == NULL) > SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid); > else > close_shroot(cfid); > -- > 2.13.6 >
diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c index 1f900b81c34a..3d59614cbe8f 100644 --- a/fs/cifs/smb2inode.c +++ b/fs/cifs/smb2inode.c @@ -511,7 +511,6 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, int rc; struct smb2_file_all_info *smb2_data; __u32 create_options = 0; - bool no_cached_open = tcon->nohandlecache; struct cifsFileInfo *cfile; struct cached_fid *cfid = NULL; @@ -524,23 +523,22 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, return -ENOMEM; /* If it is a root and its handle is cached then use it */ - if (!strlen(full_path) && !no_cached_open) { + if (!strlen(full_path)) { rc = open_shroot(xid, tcon, cifs_sb, &cfid); - if (rc) - goto out; - - if (tcon->crfid.file_all_info_is_valid) { - move_smb2_info_to_cifs(data, + if (!rc) { + if (tcon->crfid.file_all_info_is_valid) { + move_smb2_info_to_cifs(data, &tcon->crfid.file_all_info); - } else { - rc = SMB2_query_info(xid, tcon, + } else { + rc = SMB2_query_info(xid, tcon, cfid->fid->persistent_fid, cfid->fid->volatile_fid, smb2_data); - if (!rc) - move_smb2_info_to_cifs(data, smb2_data); + if (!rc) + move_smb2_info_to_cifs(data, smb2_data); + } + close_shroot(cfid); + goto out; } - close_shroot(cfid); - goto out; } cifs_get_readable_path(tcon, full_path, &cfile); diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index f5087295424c..7ee6926153b8 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -746,6 +746,9 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, u8 oplock = SMB2_OPLOCK_LEVEL_II; struct cifs_fid *pfid; + if (tcon->nohandlecache) + return -ENOTSUPP; + mutex_lock(&tcon->crfid.fid_mutex); if (tcon->crfid.is_valid) { cifs_dbg(FYI, "found a cached root file handle\n"); @@ -914,7 +917,6 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, u8 oplock = SMB2_OPLOCK_LEVEL_NONE; struct cifs_open_parms oparms; struct cifs_fid fid; - bool no_cached_open = tcon->nohandlecache; struct cached_fid *cfid = NULL; oparms.tcon = tcon; @@ -924,14 +926,12 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - if (no_cached_open) { + rc = open_shroot(xid, tcon, cifs_sb, &cfid); + if (rc == 0) + memcpy(&fid, cfid->fid, sizeof(struct cifs_fid)); + else rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL, NULL, NULL); - } else { - rc = open_shroot(xid, tcon, cifs_sb, &cfid); - if (rc == 0) - memcpy(&fid, cfid->fid, sizeof(struct cifs_fid)); - } if (rc) return; @@ -945,7 +945,7 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon, FS_VOLUME_INFORMATION); SMB2_QFS_attr(xid, tcon, fid.persistent_fid, fid.volatile_fid, FS_SECTOR_SIZE_INFORMATION); /* SMB3 specific */ - if (no_cached_open) + if (cfid == NULL) SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid); else close_shroot(cfid);
instead of doing it in the callsites for open_shroot. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/smb2inode.c | 24 +++++++++++------------- fs/cifs/smb2ops.c | 16 ++++++++-------- 2 files changed, 19 insertions(+), 21 deletions(-)