Skip to content

Commit 6d03998

Browse files
Paulo Alcantarasmfrench
authored andcommitted
smb: client: stop revalidating reparse points unnecessarily
Query dir responses don't provide enough information on reparse points such as major/minor numbers and symlink targets other than reparse tags, however we don't need to unconditionally revalidate them only because they are reparse points. Instead, revalidate them only when their ctime or reparse tag has changed. For instance, Windows Server updates ctime of reparse points when their data have changed. Signed-off-by: Paulo Alcantara (SUSE) <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 6ebfede commit 6d03998

File tree

3 files changed

+57
-81
lines changed

3 files changed

+57
-81
lines changed

fs/smb/client/cifsglob.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,6 +1565,7 @@ struct cifsInodeInfo {
15651565
spinlock_t deferred_lock; /* protection on deferred list */
15661566
bool lease_granted; /* Flag to indicate whether lease or oplock is granted. */
15671567
char *symlink_target;
1568+
__u32 reparse_tag;
15681569
};
15691570

15701571
static inline struct cifsInodeInfo *

fs/smb/client/inode.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
182182
inode->i_mode = fattr->cf_mode;
183183

184184
cifs_i->cifsAttrs = fattr->cf_cifsattrs;
185+
cifs_i->reparse_tag = fattr->cf_cifstag;
185186

186187
if (fattr->cf_flags & CIFS_FATTR_NEED_REVAL)
187188
cifs_i->time = 0;
@@ -209,7 +210,7 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
209210
inode->i_blocks = (512 - 1 + fattr->cf_bytes) >> 9;
210211
}
211212

212-
if (S_ISLNK(fattr->cf_mode)) {
213+
if (S_ISLNK(fattr->cf_mode) && fattr->cf_symlink_target) {
213214
kfree(cifs_i->symlink_target);
214215
cifs_i->symlink_target = fattr->cf_symlink_target;
215216
fattr->cf_symlink_target = NULL;
@@ -1120,6 +1121,7 @@ static int reparse_info_to_fattr(struct cifs_open_info_data *data,
11201121
else
11211122
cifs_open_info_to_fattr(fattr, data, sb);
11221123
out:
1124+
fattr->cf_cifstag = data->reparse.tag;
11231125
free_rsp_buf(rsp_buftype, rsp_iov.iov_base);
11241126
return rc;
11251127
}

fs/smb/client/readdir.c

Lines changed: 53 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,23 @@ static inline void dump_cifs_file_struct(struct file *file, char *label)
5555
}
5656
#endif /* DEBUG2 */
5757

58+
/*
59+
* Match a reparse point inode if reparse tag and ctime haven't changed.
60+
*
61+
* Windows Server updates ctime of reparse points when their data have changed.
62+
* The server doesn't allow changing reparse tags from existing reparse points,
63+
* though it's worth checking.
64+
*/
65+
static inline bool reparse_inode_match(struct inode *inode,
66+
struct cifs_fattr *fattr)
67+
{
68+
struct timespec64 ctime = inode_get_ctime(inode);
69+
70+
return (CIFS_I(inode)->cifsAttrs & ATTR_REPARSE) &&
71+
CIFS_I(inode)->reparse_tag == fattr->cf_cifstag &&
72+
timespec64_equal(&ctime, &fattr->cf_ctime);
73+
}
74+
5875
/*
5976
* Attempt to preload the dcache with the results from the FIND_FIRST/NEXT
6077
*
@@ -71,6 +88,7 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
7188
struct super_block *sb = parent->d_sb;
7289
struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
7390
DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
91+
int rc;
7492

7593
cifs_dbg(FYI, "%s: for %s\n", __func__, name->name);
7694

@@ -82,9 +100,11 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
82100
* We'll end up doing an on the wire call either way and
83101
* this spares us an invalidation.
84102
*/
85-
if (fattr->cf_flags & CIFS_FATTR_NEED_REVAL)
86-
return;
87103
retry:
104+
if ((fattr->cf_cifsattrs & ATTR_REPARSE) ||
105+
(fattr->cf_flags & CIFS_FATTR_NEED_REVAL))
106+
return;
107+
88108
dentry = d_alloc_parallel(parent, name, &wq);
89109
}
90110
if (IS_ERR(dentry))
@@ -104,12 +124,34 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
104124
if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM))
105125
fattr->cf_uniqueid = CIFS_I(inode)->uniqueid;
106126

107-
/* update inode in place
108-
* if both i_ino and i_mode didn't change */
109-
if (CIFS_I(inode)->uniqueid == fattr->cf_uniqueid &&
110-
cifs_fattr_to_inode(inode, fattr) == 0) {
111-
dput(dentry);
112-
return;
127+
/*
128+
* Update inode in place if both i_ino and i_mode didn't
129+
* change.
130+
*/
131+
if (CIFS_I(inode)->uniqueid == fattr->cf_uniqueid) {
132+
/*
133+
* Query dir responses don't provide enough
134+
* information about reparse points other than
135+
* their reparse tags. Save an invalidation by
136+
* not clobbering the existing mode, size and
137+
* symlink target (if any) when reparse tag and
138+
* ctime haven't changed.
139+
*/
140+
rc = 0;
141+
if (fattr->cf_cifsattrs & ATTR_REPARSE) {
142+
if (likely(reparse_inode_match(inode, fattr))) {
143+
fattr->cf_mode = inode->i_mode;
144+
fattr->cf_eof = CIFS_I(inode)->server_eof;
145+
fattr->cf_symlink_target = NULL;
146+
} else {
147+
CIFS_I(inode)->time = 0;
148+
rc = -ESTALE;
149+
}
150+
}
151+
if (!rc && !cifs_fattr_to_inode(inode, fattr)) {
152+
dput(dentry);
153+
return;
154+
}
113155
}
114156
}
115157
d_invalidate(dentry);
@@ -127,29 +169,6 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
127169
dput(dentry);
128170
}
129171

130-
static bool reparse_file_needs_reval(const struct cifs_fattr *fattr)
131-
{
132-
if (!(fattr->cf_cifsattrs & ATTR_REPARSE))
133-
return false;
134-
/*
135-
* The DFS tags should be only intepreted by server side as per
136-
* MS-FSCC 2.1.2.1, but let's include them anyway.
137-
*
138-
* Besides, if cf_cifstag is unset (0), then we still need it to be
139-
* revalidated to know exactly what reparse point it is.
140-
*/
141-
switch (fattr->cf_cifstag) {
142-
case IO_REPARSE_TAG_DFS:
143-
case IO_REPARSE_TAG_DFSR:
144-
case IO_REPARSE_TAG_SYMLINK:
145-
case IO_REPARSE_TAG_NFS:
146-
case IO_REPARSE_TAG_MOUNT_POINT:
147-
case 0:
148-
return true;
149-
}
150-
return false;
151-
}
152-
153172
static void
154173
cifs_fill_common_info(struct cifs_fattr *fattr, struct cifs_sb_info *cifs_sb)
155174
{
@@ -181,14 +200,6 @@ cifs_fill_common_info(struct cifs_fattr *fattr, struct cifs_sb_info *cifs_sb)
181200
}
182201

183202
out_reparse:
184-
/*
185-
* We need to revalidate it further to make a decision about whether it
186-
* is a symbolic link, DFS referral or a reparse point with a direct
187-
* access like junctions, deduplicated files, NFS symlinks.
188-
*/
189-
if (reparse_file_needs_reval(fattr))
190-
fattr->cf_flags |= CIFS_FATTR_NEED_REVAL;
191-
192203
/* non-unix readdir doesn't provide nlink */
193204
fattr->cf_flags |= CIFS_FATTR_UNKNOWN_NLINK;
194205

@@ -269,9 +280,6 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info,
269280
fattr->cf_dtype = DT_REG;
270281
}
271282

272-
if (reparse_file_needs_reval(fattr))
273-
fattr->cf_flags |= CIFS_FATTR_NEED_REVAL;
274-
275283
sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER);
276284
sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP);
277285
}
@@ -331,38 +339,6 @@ cifs_std_info_to_fattr(struct cifs_fattr *fattr, FIND_FILE_STANDARD_INFO *info,
331339
cifs_fill_common_info(fattr, cifs_sb);
332340
}
333341

334-
/* BB eventually need to add the following helper function to
335-
resolve NT_STATUS_STOPPED_ON_SYMLINK return code when
336-
we try to do FindFirst on (NTFS) directory symlinks */
337-
/*
338-
int get_symlink_reparse_path(char *full_path, struct cifs_sb_info *cifs_sb,
339-
unsigned int xid)
340-
{
341-
__u16 fid;
342-
int len;
343-
int oplock = 0;
344-
int rc;
345-
struct cifs_tcon *ptcon = cifs_sb_tcon(cifs_sb);
346-
char *tmpbuffer;
347-
348-
rc = CIFSSMBOpen(xid, ptcon, full_path, FILE_OPEN, GENERIC_READ,
349-
OPEN_REPARSE_POINT, &fid, &oplock, NULL,
350-
cifs_sb->local_nls,
351-
cifs_remap(cifs_sb);
352-
if (!rc) {
353-
tmpbuffer = kmalloc(maxpath);
354-
rc = CIFSSMBQueryReparseLinkInfo(xid, ptcon, full_path,
355-
tmpbuffer,
356-
maxpath -1,
357-
fid,
358-
cifs_sb->local_nls);
359-
if (CIFSSMBClose(xid, ptcon, fid)) {
360-
cifs_dbg(FYI, "Error closing temporary reparsepoint open\n");
361-
}
362-
}
363-
}
364-
*/
365-
366342
static int
367343
_initiate_cifs_search(const unsigned int xid, struct file *file,
368344
const char *full_path)
@@ -431,13 +407,10 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
431407
&cifsFile->fid, search_flags,
432408
&cifsFile->srch_inf);
433409

434-
if (rc == 0)
410+
if (rc == 0) {
435411
cifsFile->invalidHandle = false;
436-
/* BB add following call to handle readdir on new NTFS symlink errors
437-
else if STATUS_STOPPED_ON_SYMLINK
438-
call get_symlink_reparse_path and retry with new path */
439-
else if ((rc == -EOPNOTSUPP) &&
440-
(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
412+
} else if ((rc == -EOPNOTSUPP) &&
413+
(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
441414
cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
442415
goto ffirst_retry;
443416
}

0 commit comments

Comments
 (0)